Skip to contentSkip to conversation

cpufreq: mt8173: add CPUFREQ_HAVE_GOVERNOR_PER_POLICY flag

PATCH New Pi-Cheng Chen 4 participants 15 messages Nov 29, 2015 – Dec 14, 2015

Subsystems (2)
Arm MediaTek CPU frequency scaling framework
lore
Share & tools
lore

Pi-Cheng Chen

author
#0 lore raw

PATCH 1/4 cpufreq: mt8173: add CPUFREQ_HAVE_GOVERNOR_PER_POLICY flag

Add CPUFREQ_HAVE_GOVERNOR_PER_POLICY to have individual set of tunables for each cluster of MT8173.

Signed-off-by Pi-Cheng Chen <[email protected]>
---
 drivers/cpufreq/mt8173-cpufreq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/cpufreq/mt8173-cpufreq.c +2 -1
index 83001dc..c438109 100644
@@ -469,7 +469,8 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
 }
 
 static struct cpufreq_driver mt8173_cpufreq_driver = {
-	.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+	.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+		 CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
 	.verify = cpufreq_generic_frequency_table_verify,
 	.target_index = mtk_cpufreq_set_target,
 	.get = cpufreq_generic_get,
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 2/4 cpufreq: mt8173: remove redundant regulator_get_voltage() call

Remove redundant regulator_get_voltage() call to get Vsram value since it will be obtained later at the beginning of voltage tracking loop.

Signed-off-by Pi-Cheng Chen <[email protected]>
---
 drivers/cpufreq/mt8173-cpufreq.c | 1 -
 1 file changed, 1 deletion(-)
drivers/cpufreq/mt8173-cpufreq.c +0 -1
index c438109..750cda7 100644
@@ -59,7 +59,6 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 	int old_vproc, old_vsram, new_vsram, vsram, vproc, ret;
 
 	old_vproc = regulator_get_voltage(proc_reg);
-	old_vsram = regulator_get_voltage(sram_reg);
 	/* Vsram should not exceed the maximum allowed voltage of SoC. */
 	new_vsram = min(new_vproc + MIN_VOLT_SHIFT, MAX_VOLT_LIMIT);
Signature · 1 line
1.9.1
maintainer
#2 lore raw

PATCH 2/4 cpufreq: mt8173: remove redundant regulator_get_voltage() call

Quoted 14 lines · Pi-Cheng Chen

Remove redundant regulator_get_voltage() call to get Vsram value since it will be obtained later at the beginning of voltage tracking loop.

Signed-off-by Pi-Cheng Chen <[email protected]>
---
 drivers/cpufreq/mt8173-cpufreq.c | 1 -
 1 file changed, 1 deletion(-)
drivers/cpufreq/mt8173-cpufreq.c +0 -0
index c438109..750cda7 100644

drivers/cpufreq/mt8173-cpufreq.c static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info, · line 64

Quoted 7 lines
drivers/cpufreq/mt8173-cpufreq.c +0 -1
@@ -59,7 +59,6 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 	int old_vproc, old_vsram, new_vsram, vsram, vproc, ret;
 
 	old_vproc = regulator_get_voltage(proc_reg);
-	old_vsram = regulator_get_voltage(sram_reg);
 	/* Vsram should not exceed the maximum allowed voltage of SoC. */
 	new_vsram = min(new_vproc + MIN_VOLT_SHIFT, MAX_VOLT_LIMIT);
Acked-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh
author
#3 lore raw

PATCH 3/4 cpufreq: mt8173: check return value of regulator_get_voltage() call

In reply to Pi-Cheng Chen

Sometimes regulator_get_voltage() call returns negative values for reasons(e.g. underlying I2C bus timeout). Add check for the return values and fail out early.

Signed-off-by Pi-Cheng Chen <[email protected]>
---
 drivers/cpufreq/mt8173-cpufreq.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
drivers/cpufreq/mt8173-cpufreq.c +28 -0
index 750cda7..9d0fe37 100644
@@ -59,6 +59,10 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 	int old_vproc, old_vsram, new_vsram, vsram, vproc, ret;
 
 	old_vproc = regulator_get_voltage(proc_reg);
+	if (old_vproc < 0) {
+		pr_err("%s: invalid Vproc value: %d\n", __func__, old_vproc);
+		return old_vproc;
+	}
 	/* Vsram should not exceed the maximum allowed voltage of SoC. */
 	new_vsram = min(new_vproc + MIN_VOLT_SHIFT, MAX_VOLT_LIMIT);
 

@@ -71,7 +75,17 @@ 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);
+				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);
+				return old_vproc;
+			}
 
 			vsram = min(new_vsram, old_vproc + MAX_VOLT_SHIFT);
 

@@ -116,7 +130,17 @@ 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);
+				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);
+				return old_vsram;
+			}
 
 			vproc = max(new_vproc, old_vsram - MAX_VOLT_SHIFT);
 			ret = regulator_set_voltage(proc_reg, vproc,

@@ -184,6 +208,10 @@ 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);
+		return old_vproc;
+	}
 
 	freq_hz = freq_table[index].frequency * 1000;
Signature · 1 line
1.9.1
maintainer
#4 lore raw

PATCH 3/4 cpufreq: mt8173: check return value of regulator_get_voltage() call

Quoted 61 lines · Pi-Cheng Chen

Sometimes regulator_get_voltage() call returns negative values for reasons(e.g. underlying I2C bus timeout). Add check for the return values and fail out early.

Signed-off-by Pi-Cheng Chen <[email protected]>
---
 drivers/cpufreq/mt8173-cpufreq.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
drivers/cpufreq/mt8173-cpufreq.c +24 -0
index 750cda7..9d0fe37 100644
@@ -59,6 +59,10 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 	int old_vproc, old_vsram, new_vsram, vsram, vproc, ret;
 
 	old_vproc = regulator_get_voltage(proc_reg);
+	if (old_vproc < 0) {
+		pr_err("%s: invalid Vproc value: %d\n", __func__, old_vproc);
+		return old_vproc;
+	}
 	/* Vsram should not exceed the maximum allowed voltage of SoC. */
 	new_vsram = min(new_vproc + MIN_VOLT_SHIFT, MAX_VOLT_LIMIT);
 

@@ -71,7 +75,17 @@ 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);
+				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);
+				return old_vproc;
+			}
 
 			vsram = min(new_vsram, old_vproc + MAX_VOLT_SHIFT);
 

@@ -116,7 +130,17 @@ 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);
+				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);
+				return old_vsram;
+			}
 
 			vproc = max(new_vproc, old_vsram - MAX_VOLT_SHIFT);
 			ret = regulator_set_voltage(proc_reg, vproc,

drivers/cpufreq/mt8173-cpufreq.c static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, · line 216

Quoted 9 lines
drivers/cpufreq/mt8173-cpufreq.c +4 -0
@@ -184,6 +208,10 @@ 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);
+		return old_vproc;
+	}
 
 	freq_hz = freq_table[index].frequency * 1000;
Acked-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh
author
#5 lore raw

PATCH 4/4 cpufreq: mt8173: move resources allocation into ->probe()

In reply to Pi-Cheng Chen

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>
---
 drivers/cpufreq/mt8173-cpufreq.c | 94 ++++++++++++++++++++++++++++------------
 1 file changed, 67 insertions(+), 27 deletions(-)
drivers/cpufreq/mt8173-cpufreq.c +67 -27
index 9d0fe37..1142625 100644
@@ -41,16 +41,35 @@
  * the original PLL becomes stable at target frequency.
  */
 struct mtk_cpu_dvfs_info {
+	struct cpumask cpus;
 	struct device *cpu_dev;
 	struct regulator *proc_reg;
 	struct regulator *sram_reg;
 	struct clk *cpu_clk;
 	struct clk *inter_clk;
 	struct thermal_cooling_device *cdev;
+	struct list_head list_head;
 	int intermediate_voltage;
 	bool need_voltage_tracking;
 };
 
+static LIST_HEAD(dvfs_info_list);
+
+struct mtk_cpu_dvfs_info *get_dvfs_info(int cpu)
+{
+	struct mtk_cpu_dvfs_info *info;
+	struct list_head *list;
+
+	list_for_each(list, &dvfs_info_list) {
+		info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+		if (cpumask_test_cpu(cpu, &info->cpus))
+			return info;
+	}
+
+	return NULL;
+}
+
 static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 					int new_vproc)
 {

@@ -402,6 +421,9 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	 */
 	info->need_voltage_tracking = !IS_ERR(sram_reg);
 
+	/* CPUs in the same cluster share a clock and power domain. */
+	cpumask_copy(&info->cpus, &cpu_topology[cpu].core_sibling);
+
 	return 0;
 
 out_free_opp_table:

@@ -440,47 +462,32 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
 	struct cpufreq_frequency_table *freq_table;
 	int ret;
 
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
-
-	ret = mtk_cpu_dvfs_info_init(info, policy->cpu);
-	if (ret) {
-		pr_err("%s failed to initialize dvfs info for cpu%d\n",
-		       __func__, policy->cpu);
-		goto out_free_dvfs_info;
+	info = get_dvfs_info(policy->cpu);
+	if (!info) {
+		pr_err("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);
-		goto out_release_dvfs_info;
+		return ret;
 	}
 
 	ret = cpufreq_table_validate_and_show(policy, freq_table);
 	if (ret) {
 		pr_err("%s: invalid frequency table: %d\n", __func__, ret);
-		goto out_free_cpufreq_table;
+		dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
+		return ret;
 	}
 
-	/* CPUs in the same cluster share a clock and power domain. */
-	cpumask_copy(policy->cpus, &cpu_topology[policy->cpu].core_sibling);
+	cpumask_copy(policy->cpus, &info->cpus);
 	policy->driver_data = info;
 	policy->clk = info->cpu_clk;
 
 	return 0;
-
-out_free_cpufreq_table:
-	dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
-
-out_release_dvfs_info:
-	mtk_cpu_dvfs_info_release(info);
-
-out_free_dvfs_info:
-	kfree(info);
-
-	return ret;
 }
 
 static int mtk_cpufreq_exit(struct cpufreq_policy *policy)

@@ -489,8 +496,6 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
 
 	cpufreq_cooling_unregister(info->cdev);
 	dev_pm_opp_free_cpufreq_table(info->cpu_dev, &policy->freq_table);
-	mtk_cpu_dvfs_info_release(info);
-	kfree(info);
 
 	return 0;
 }

@@ -510,12 +515,47 @@ static struct cpufreq_driver mt8173_cpufreq_driver = {
 
 static int mt8173_cpufreq_probe(struct platform_device *pdev)
 {
-	int ret;
+	struct mtk_cpu_dvfs_info *info;
+	struct list_head *list, *tmp;
+	int cpu, ret;
+
+	for_each_possible_cpu(cpu) {
+		info = get_dvfs_info(cpu);
+		if (info)
+			continue;
+
+		info = kzalloc(sizeof(*info), GFP_KERNEL);
+		if (!info) {
+			ret = -ENOMEM;
+			goto release_dvfs_info_list;
+		}
+
+		ret = mtk_cpu_dvfs_info_init(info, cpu);
+		if (ret) {
+			pr_err("%s failed to initialize dvfs info for cpu%d\n",
+			       __func__, cpu);
+			kfree(info);
+			goto release_dvfs_info_list;
+		}
+
+		list_add(&info->list_head, &dvfs_info_list);
+	}
 
 	ret = cpufreq_register_driver(&mt8173_cpufreq_driver);
 	if (ret)
 		pr_err("failed to register mtk cpufreq driver\n");
 
+	return 0;
+
+release_dvfs_info_list:
+	list_for_each_safe(list, tmp, &dvfs_info_list) {
+		info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+		mtk_cpu_dvfs_info_release(info);
+		list_del(list);
+		kfree(info);
+	}
+
 	return ret;
 }
Signature · 1 line
1.9.1
author
#6 lore raw

PATCH v2 1/1 cpufreq: mt8173: move resources allocation into ->probe()

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>

---
v1->v2:
- fix error handling path in ->probe()

---
 drivers/cpufreq/mt8173-cpufreq.c | 98 ++++++++++++++++++++++++++++------------
 1 file changed, 70 insertions(+), 28 deletions(-)
drivers/cpufreq/mt8173-cpufreq.c +70 -28
index 9d0fe37..257bcb9 100644
@@ -41,16 +41,35 @@
  * the original PLL becomes stable at target frequency.
  */
 struct mtk_cpu_dvfs_info {
+	struct cpumask cpus;
 	struct device *cpu_dev;
 	struct regulator *proc_reg;
 	struct regulator *sram_reg;
 	struct clk *cpu_clk;
 	struct clk *inter_clk;
 	struct thermal_cooling_device *cdev;
+	struct list_head list_head;
 	int intermediate_voltage;
 	bool need_voltage_tracking;
 };
 
+static LIST_HEAD(dvfs_info_list);
+
+struct mtk_cpu_dvfs_info *get_dvfs_info(int cpu)
+{
+	struct mtk_cpu_dvfs_info *info;
+	struct list_head *list;
+
+	list_for_each(list, &dvfs_info_list) {
+		info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+		if (cpumask_test_cpu(cpu, &info->cpus))
+			return info;
+	}
+
+	return NULL;
+}
+
 static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 					int new_vproc)
 {

@@ -402,6 +421,9 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	 */
 	info->need_voltage_tracking = !IS_ERR(sram_reg);
 
+	/* CPUs in the same cluster share a clock and power domain. */
+	cpumask_copy(&info->cpus, &cpu_topology[cpu].core_sibling);
+
 	return 0;
 
 out_free_opp_table:

@@ -440,47 +462,32 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
 	struct cpufreq_frequency_table *freq_table;
 	int ret;
 
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
-
-	ret = mtk_cpu_dvfs_info_init(info, policy->cpu);
-	if (ret) {
-		pr_err("%s failed to initialize dvfs info for cpu%d\n",
-		       __func__, policy->cpu);
-		goto out_free_dvfs_info;
+	info = get_dvfs_info(policy->cpu);
+	if (!info) {
+		pr_err("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);
-		goto out_release_dvfs_info;
+		return ret;
 	}
 
 	ret = cpufreq_table_validate_and_show(policy, freq_table);
 	if (ret) {
 		pr_err("%s: invalid frequency table: %d\n", __func__, ret);
-		goto out_free_cpufreq_table;
+		dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
+		return ret;
 	}
 
-	/* CPUs in the same cluster share a clock and power domain. */
-	cpumask_copy(policy->cpus, &cpu_topology[policy->cpu].core_sibling);
+	cpumask_copy(policy->cpus, &info->cpus);
 	policy->driver_data = info;
 	policy->clk = info->cpu_clk;
 
 	return 0;
-
-out_free_cpufreq_table:
-	dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
-
-out_release_dvfs_info:
-	mtk_cpu_dvfs_info_release(info);
-
-out_free_dvfs_info:
-	kfree(info);
-
-	return ret;
 }
 
 static int mtk_cpufreq_exit(struct cpufreq_policy *policy)

@@ -489,8 +496,6 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
 
 	cpufreq_cooling_unregister(info->cdev);
 	dev_pm_opp_free_cpufreq_table(info->cpu_dev, &policy->freq_table);
-	mtk_cpu_dvfs_info_release(info);
-	kfree(info);
 
 	return 0;
 }

@@ -510,11 +515,48 @@ static struct cpufreq_driver mt8173_cpufreq_driver = {
 
 static int mt8173_cpufreq_probe(struct platform_device *pdev)
 {
-	int ret;
+	struct mtk_cpu_dvfs_info *info;
+	struct list_head *list, *tmp;
+	int cpu, ret;
+
+	for_each_possible_cpu(cpu) {
+		info = get_dvfs_info(cpu);
+		if (info)
+			continue;
+
+		info = kzalloc(sizeof(*info), GFP_KERNEL);
+		if (!info) {
+			ret = -ENOMEM;
+			goto release_dvfs_info_list;
+		}
+
+		ret = mtk_cpu_dvfs_info_init(info, cpu);
+		if (ret) {
+			pr_err("%s failed to initialize dvfs info for cpu%d\n",
+			       __func__, cpu);
+			kfree(info);
+			goto release_dvfs_info_list;
+		}
+
+		list_add(&info->list_head, &dvfs_info_list);
+	}
 
 	ret = cpufreq_register_driver(&mt8173_cpufreq_driver);
-	if (ret)
+	if (ret) {
 		pr_err("failed to register mtk cpufreq driver\n");
+		goto release_dvfs_info_list;
+	}
+
+	return 0;
+
+release_dvfs_info_list:
+	list_for_each_safe(list, tmp, &dvfs_info_list) {
+		info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+		mtk_cpu_dvfs_info_release(info);
+		list_del(list);
+		kfree(info);
+	}
 
 	return ret;
 }
Signature · 1 line
1.9.1
maintainer
#7 lore raw

PATCH v2 1/1 cpufreq: mt8173: move resources allocation into ->probe()

Quoted 8 lines · Pi-Cheng Chen

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>

---
v1->v2:
- fix error handling path in ->probe()

Acked-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh
#8 lore raw

PATCH v2 1/1 cpufreq: mt8173: move resources allocation into ->probe()

In reply to Pi-Cheng Chen

Hi Pi-Cheng,

Quoted 23 lines · Pi-Cheng Chen

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>

---
v1->v2:
- fix error handling path in ->probe()

---
 drivers/cpufreq/mt8173-cpufreq.c | 98 ++++++++++++++++++++++++++++------------
 1 file changed, 70 insertions(+), 28 deletions(-)
drivers/cpufreq/mt8173-cpufreq.c +1 -0
index 9d0fe37..257bcb9 100644
@@ -41,16 +41,35 @@
  * the original PLL becomes stable at target frequency.
  */
 struct mtk_cpu_dvfs_info {
+       struct cpumask cpus;
        struct device *cpu_dev;
        struct regulator *proc_reg;

drivers/cpufreq/mt8173-cpufreq.c line 56

Quoted 10 lines
drivers/cpufreq/mt8173-cpufreq.c +2 -0
        struct regulator *sram_reg;
        struct clk *cpu_clk;
        struct clk *inter_clk;
        struct thermal_cooling_device *cdev;
+       struct list_head list_head;
        int intermediate_voltage;
        bool need_voltage_tracking;
 };
 
+static LIST_HEAD(dvfs_info_list);

This struct is specific to this driver, so:

mtk_cpu_dvfs_info_list

drivers/cpufreq/mt8173-cpufreq.c line 58

Quoted 2 lines
drivers/cpufreq/mt8173-cpufreq.c +2 -0
+
+struct mtk_cpu_dvfs_info *get_dvfs_info(int cpu)

static

Also, it would be clearer to name this function something like "mtk_cpu_dfs_info_lookup" since "mtk_cpu_dfs_info" is the type this function looks up in the list.
It would also match the other functions such as "mtk_cpu_dvfs_info_init()".

Quoted 38 lines
drivers/cpufreq/mt8173-cpufreq.c +17 -7
+{
+       struct mtk_cpu_dvfs_info *info;
+       struct list_head *list;
+
+       list_for_each(list, &dvfs_info_list) {
+               info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+               if (cpumask_test_cpu(cpu, &info->cpus))
+                       return info;
+       }
+
+       return NULL;
+}
+
 static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
                                        int new_vproc)
 {

@@ -402,6 +421,9 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
         */
        info->need_voltage_tracking = !IS_ERR(sram_reg);
 
+       /* CPUs in the same cluster share a clock and power domain. */
+       cpumask_copy(&info->cpus, &cpu_topology[cpu].core_sibling);
+
        return 0;
 
 out_free_opp_table:

@@ -440,47 +462,32 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
        struct cpufreq_frequency_table *freq_table;
        int ret;
 
-       info = kzalloc(sizeof(*info), GFP_KERNEL);
-       if (!info)
-               return -ENOMEM;
-
-       ret = mtk_cpu_dvfs_info_init(info, policy->cpu);
-       if (ret) {
-               pr_err("%s failed to initialize dvfs info for cpu%d\n",

drivers/cpufreq/mt8173-cpufreq.c static int mtk_cpufreq_init(struct cpufreq_policy *policy) · line 472

Quoted 10 lines
drivers/cpufreq/mt8173-cpufreq.c +5 -2
-                      __func__, policy->cpu);
-               goto out_free_dvfs_info;
+       info = get_dvfs_info(policy->cpu);
+       if (!info) {
+               pr_err("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);

Hmm. Does this change at init() time? Or can we move this to probe(), too?

drivers/cpufreq/mt8173-cpufreq.c static int mtk_cpufreq_init(struct cpufreq_policy *policy) · line 483

Quoted 13 lines
drivers/cpufreq/mt8173-cpufreq.c +3 -2
        if (ret) {
                pr_err("failed to init cpufreq table for cpu%d: %d\n",
                       policy->cpu, ret);
-               goto out_release_dvfs_info;
+               return ret;
        }
 
        ret = cpufreq_table_validate_and_show(policy, freq_table);
        if (ret) {
                pr_err("%s: invalid frequency table: %d\n", __func__, ret);
-               goto out_free_cpufreq_table;
+               dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
+               return ret;

If dev_pm_opp_init_cpufreq_table() really is needed at init() time, then please don't change this; still use "goto out_free_cpufreq_table" here.

Quoted 33 lines
drivers/cpufreq/mt8173-cpufreq.c +1 -15
        }
 
-       /* CPUs in the same cluster share a clock and power domain. */
-       cpumask_copy(policy->cpus, &cpu_topology[policy->cpu].core_sibling);
+       cpumask_copy(policy->cpus, &info->cpus);
        policy->driver_data = info;
        policy->clk = info->cpu_clk;
 
        return 0;
-
-out_free_cpufreq_table:
-       dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
-
-out_release_dvfs_info:
-       mtk_cpu_dvfs_info_release(info);
-
-out_free_dvfs_info:
-       kfree(info);
-
-       return ret;
 }
 
 static int mtk_cpufreq_exit(struct cpufreq_policy *policy)

@@ -489,8 +496,6 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
 
        cpufreq_cooling_unregister(info->cdev);
        dev_pm_opp_free_cpufreq_table(info->cpu_dev, &policy->freq_table);
-       mtk_cpu_dvfs_info_release(info);
-       kfree(info);
 
        return 0;
 }

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 522

Quoted 9 lines
drivers/cpufreq/mt8173-cpufreq.c +5 -1
@@ -510,11 +515,48 @@ static struct cpufreq_driver mt8173_cpufreq_driver = {
 
 static int mt8173_cpufreq_probe(struct platform_device *pdev)
 {
-       int ret;
+       struct mtk_cpu_dvfs_info *info;
+       struct list_head *list, *tmp;
+       int cpu, ret;
+
+       for_each_possible_cpu(cpu) {

Is there an iterator for cpu clusters? Would be useful here.

Or, even better:
* Is there a way to way to map cpu to cpu cluster ID?
* Is there a way to count the number of clusters?
If so you can just use an array of mtk_cpu_dvfs_info, and use a cpu's cluster ID as its index into the array.

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 525

Quoted 3 lines
drivers/cpufreq/mt8173-cpufreq.c +3 -0
+               info = get_dvfs_info(cpu);
+               if (info)
+                       continue;

How could this ever return non-NULL?
This is probe; it won't be called twice - not unless a prior probe() failed (and/or after .remove()), right?

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 527

Quoted 2 lines
drivers/cpufreq/mt8173-cpufreq.c +2 -0
+
+               info = kzalloc(sizeof(*info), GFP_KERNEL);

devm_kzalloc() (and you can then remove the kfree()'s below).

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 536

Quoted 9 lines
drivers/cpufreq/mt8173-cpufreq.c +9 -0
+               if (!info) {
+                       ret = -ENOMEM;
+                       goto release_dvfs_info_list;
+               }
+
+               ret = mtk_cpu_dvfs_info_init(info, cpu);
+               if (ret) {
+                       pr_err("%s failed to initialize dvfs info for cpu%d\n",
+                              __func__, cpu);

dev_err(&pdev->dev, ) here and elsewhere.

-Dan

Quoted 30 lines
drivers/cpufreq/mt8173-cpufreq.c +20 -2
+                       kfree(info);
+                       goto release_dvfs_info_list;
+               }
+
+               list_add(&info->list_head, &dvfs_info_list);
+       }
 
        ret = cpufreq_register_driver(&mt8173_cpufreq_driver);
-       if (ret)
+       if (ret) {
                pr_err("failed to register mtk cpufreq driver\n");
+               goto release_dvfs_info_list;
+       }
+
+       return 0;
+
+release_dvfs_info_list:
+       list_for_each_safe(list, tmp, &dvfs_info_list) {
+               info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+               mtk_cpu_dvfs_info_release(info);
+               list_del(list);
+               kfree(info);
+       }
 
        return ret;
 }
--

1.9.1

author
#9 lore raw

PATCH v2 1/1 cpufreq: mt8173: move resources allocation into ->probe()

Hi Daniel,

Quoted 35 lines · Daniel Kurtz

Hi Pi-Cheng,

Quoted 32 lines · Pi-Cheng Chen

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>

---
v1->v2:
- fix error handling path in ->probe()

---
 drivers/cpufreq/mt8173-cpufreq.c | 98 ++++++++++++++++++++++++++++------------
 1 file changed, 70 insertions(+), 28 deletions(-)
drivers/cpufreq/mt8173-cpufreq.c +2 -0
index 9d0fe37..257bcb9 100644
@@ -41,16 +41,35 @@
  * the original PLL becomes stable at target frequency.
  */
 struct mtk_cpu_dvfs_info {
+       struct cpumask cpus;
        struct device *cpu_dev;
        struct regulator *proc_reg;
        struct regulator *sram_reg;
        struct clk *cpu_clk;
        struct clk *inter_clk;
        struct thermal_cooling_device *cdev;
+       struct list_head list_head;
        int intermediate_voltage;
        bool need_voltage_tracking;
 };
Quoted 5 lines

drivers/cpufreq/mt8173-cpufreq.c line 56

Quoted 1 line
drivers/cpufreq/mt8173-cpufreq.c +1 -0
+static LIST_HEAD(dvfs_info_list);

This struct is specific to this driver, so:

mtk_cpu_dvfs_info_list

That's fair. Both work for me.
But since it's a static variable, I prefer the shorter name since it won't be misunderstood.

Quoted 5 lines

drivers/cpufreq/mt8173-cpufreq.c line 58

Quoted 3 lines
drivers/cpufreq/mt8173-cpufreq.c +2 -0
+
+struct mtk_cpu_dvfs_info *get_dvfs_info(int cpu)

static

Will do it.

Quoted 5 lines

Also, it would be clearer to name this function something like "mtk_cpu_dfs_info_lookup" since "mtk_cpu_dfs_info" is the type this function looks up in the list.
It would also match the other functions such as "mtk_cpu_dvfs_info_init()".

mtk_cpu_dvfs_info_lookup() looks good to me.
WIll do it.

Quoted 48 lines
Quoted 48 lines
drivers/cpufreq/mt8173-cpufreq.c +22 -9
+{
+       struct mtk_cpu_dvfs_info *info;
+       struct list_head *list;
+
+       list_for_each(list, &dvfs_info_list) {
+               info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+               if (cpumask_test_cpu(cpu, &info->cpus))
+                       return info;
+       }
+
+       return NULL;
+}
+
 static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
                                        int new_vproc)
 {

@@ -402,6 +421,9 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
         */
        info->need_voltage_tracking = !IS_ERR(sram_reg);
 
+       /* CPUs in the same cluster share a clock and power domain. */
+       cpumask_copy(&info->cpus, &cpu_topology[cpu].core_sibling);
+
        return 0;
 
 out_free_opp_table:

@@ -440,47 +462,32 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
        struct cpufreq_frequency_table *freq_table;
        int ret;
 
-       info = kzalloc(sizeof(*info), GFP_KERNEL);
-       if (!info)
-               return -ENOMEM;
-
-       ret = mtk_cpu_dvfs_info_init(info, policy->cpu);
-       if (ret) {
-               pr_err("%s failed to initialize dvfs info for cpu%d\n",
-                      __func__, policy->cpu);
-               goto out_free_dvfs_info;
+       info = get_dvfs_info(policy->cpu);
+       if (!info) {
+               pr_err("dvfs info for cpu%d is not initialized.\n",
+                      policy->cpu);
+               return -EINVAL;
        }
Quoted 3 lines

drivers/cpufreq/mt8173-cpufreq.c static int mtk_cpufreq_init(struct cpufreq_policy *policy) · line 472

Quoted 1 line
drivers/cpufreq/mt8173-cpufreq.c +0 -0
        ret = dev_pm_opp_init_cpufreq_table(info->cpu_dev, &freq_table);

Hmm. Does this change at init() time? Or can we move this to probe(), too?

It doesn't change at init() time for MT8173 since we don't add OPP dynamically and I don't see any problem to move it. I will move it to probe().

Quoted 64 lines
Quoted 4 lines
drivers/cpufreq/mt8173-cpufreq.c +0 -0
        if (ret) {
                pr_err("failed to init cpufreq table for cpu%d: %d\n",
                       policy->cpu, ret);

drivers/cpufreq/mt8173-cpufreq.c static int mtk_cpufreq_init(struct cpufreq_policy *policy) · line 483

Quoted 10 lines
drivers/cpufreq/mt8173-cpufreq.c +3 -2
-               goto out_release_dvfs_info;
+               return ret;
        }
 
        ret = cpufreq_table_validate_and_show(policy, freq_table);
        if (ret) {
                pr_err("%s: invalid frequency table: %d\n", __func__, ret);
-               goto out_free_cpufreq_table;
+               dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
+               return ret;

If dev_pm_opp_init_cpufreq_table() really is needed at init() time, then please don't change this; still use "goto out_free_cpufreq_table" here.

Quoted 33 lines
drivers/cpufreq/mt8173-cpufreq.c +1 -15
        }
 
-       /* CPUs in the same cluster share a clock and power domain. */
-       cpumask_copy(policy->cpus, &cpu_topology[policy->cpu].core_sibling);
+       cpumask_copy(policy->cpus, &info->cpus);
        policy->driver_data = info;
        policy->clk = info->cpu_clk;
 
        return 0;
-
-out_free_cpufreq_table:
-       dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
-
-out_release_dvfs_info:
-       mtk_cpu_dvfs_info_release(info);
-
-out_free_dvfs_info:
-       kfree(info);
-
-       return ret;
 }
 
 static int mtk_cpufreq_exit(struct cpufreq_policy *policy)

@@ -489,8 +496,6 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
 
        cpufreq_cooling_unregister(info->cdev);
        dev_pm_opp_free_cpufreq_table(info->cpu_dev, &policy->freq_table);
-       mtk_cpu_dvfs_info_release(info);
-       kfree(info);
 
        return 0;
 }

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 522

Quoted 9 lines
drivers/cpufreq/mt8173-cpufreq.c +5 -1
@@ -510,11 +515,48 @@ static struct cpufreq_driver mt8173_cpufreq_driver = {
 
 static int mt8173_cpufreq_probe(struct platform_device *pdev)
 {
-       int ret;
+       struct mtk_cpu_dvfs_info *info;
+       struct list_head *list, *tmp;
+       int cpu, ret;
+
+       for_each_possible_cpu(cpu) {

Is there an iterator for cpu clusters? Would be useful here.

Quoted 5 lines

Or, even better:
* Is there a way to way to map cpu to cpu cluster ID?
* Is there a way to count the number of clusters?
If so you can just use an array of mtk_cpu_dvfs_info, and use a cpu's cluster ID as its index into the array.

As I originally planned, this driver should be easily extended support for other
Mediatek platforms. For this reason, we should not assume there's a fixed mapping for CPU ID and cluster ID and the CPU DVFS domain and cluster neither. Furthermore, the CPU DVFS domain sharing information will be provided by operating-points-v2 bindings on which I am working to migrate mt8173-cpufreq driver. We should not put the same information in two different places.

Quoted 8 lines

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 525

Quoted 4 lines
drivers/cpufreq/mt8173-cpufreq.c +3 -0
+               info = get_dvfs_info(cpu);
+               if (info)
+                       continue;

How could this ever return non-NULL?
This is probe; it won't be called twice - not unless a prior probe() failed (and/or after .remove()), right?

Yes. it does return non-NULL for CPU1 and CPU3.
When it goes into the loop with cpu=0, mtk_cpu_dvfs_info_init() will copy the cluster core_sibling mask to info->cpus:

cpumask_copy(&info->cpus, &cpu_topology[cpu].core_sibling);

which means cpu0 and cpu1 share this struct mtk_cpu_dvfs_info variable. So when it goes into the loop with cpu=1, get_dvfs_info() will return the shared data structure. Similar for CPU2 and CPU3.

BTW, the shared information from core_sibling cpumask will be replaced by operating-points-v2 bindings later.

Quoted 5 lines

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 527

Quoted 3 lines
drivers/cpufreq/mt8173-cpufreq.c +2 -0
+
+               info = kzalloc(sizeof(*info), GFP_KERNEL);

devm_kzalloc() (and you can then remove the kfree()'s below).

Yes. Will do it.

Quoted 11 lines

drivers/cpufreq/mt8173-cpufreq.c static struct cpufreq_driver mt8173_cpufreq_driver = { · line 536

Quoted 10 lines
drivers/cpufreq/mt8173-cpufreq.c +9 -0
+               if (!info) {
+                       ret = -ENOMEM;
+                       goto release_dvfs_info_list;
+               }
+
+               ret = mtk_cpu_dvfs_info_init(info, cpu);
+               if (ret) {
+                       pr_err("%s failed to initialize dvfs info for cpu%d\n",
+                              __func__, cpu);
Quoted 1 line

dev_err(&pdev->dev, ) here and elsewhere.

Will do it.
Thanks for reviewing.

Pi-Cheng

Quoted 33 lines

-Dan

Quoted 30 lines
drivers/cpufreq/mt8173-cpufreq.c +20 -2
+                       kfree(info);
+                       goto release_dvfs_info_list;
+               }
+
+               list_add(&info->list_head, &dvfs_info_list);
+       }
 
        ret = cpufreq_register_driver(&mt8173_cpufreq_driver);
-       if (ret)
+       if (ret) {
                pr_err("failed to register mtk cpufreq driver\n");
+               goto release_dvfs_info_list;
+       }
+
+       return 0;
+
+release_dvfs_info_list:
+       list_for_each_safe(list, tmp, &dvfs_info_list) {
+               info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+               mtk_cpu_dvfs_info_release(info);
+               list_del(list);
+               kfree(info);
+       }
 
        return ret;
 }
--

1.9.1

maintainer
#10 lore raw

PATCH v2 1/1 cpufreq: mt8173: move resources allocation into ->probe()

Quoted 6 lines · Pi-Cheng Chen
Quoted 3 lines
Quoted 1 line
        ret = dev_pm_opp_init_cpufreq_table(info->cpu_dev, &freq_table);

Hmm. Does this change at init() time? Or can we move this to probe(), too?

It doesn't change at init() time for MT8173 since we don't add OPP dynamically and I don't see any problem to move it. I will move it to probe().

No. None of this belongs to probe(), we do it there just to handle the EPROBE_DEFER thing. Please don't move anything that isn't required to be.

Signature · 1 line
viresh
#11 lore raw

PATCH v2 1/1 cpufreq: mt8173: move resources allocation into ->probe()

In reply to Pi-Cheng Chen
Quoted 5 lines · Pi-Cheng Chen

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>

I'm expecting a v3 here. Please let me know if I should apply this instead.

Thanks,
Rafael

author
#12 lore raw

PATCH v3 1/1 cpufreq: mt8173: move resources allocation into ->probe()

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>
Acked-by Viresh Kumar <[email protected]>

---
v2->v3:
- rename get_dvfs_info() as mtk_cpu_dvfs_info_lookup()
- fix some minor issues of v2
v1->v2:
- fix error handling path in ->probe()

---
 drivers/cpufreq/mt8173-cpufreq.c | 92 +++++++++++++++++++++++++++++-----------
 1 file changed, 68 insertions(+), 24 deletions(-)
drivers/cpufreq/mt8173-cpufreq.c +68 -24
index 9d0fe37..fd601b9 100644
@@ -41,16 +41,35 @@
  * the original PLL becomes stable at target frequency.
  */
 struct mtk_cpu_dvfs_info {
+	struct cpumask cpus;
 	struct device *cpu_dev;
 	struct regulator *proc_reg;
 	struct regulator *sram_reg;
 	struct clk *cpu_clk;
 	struct clk *inter_clk;
 	struct thermal_cooling_device *cdev;
+	struct list_head list_head;
 	int intermediate_voltage;
 	bool need_voltage_tracking;
 };
 
+static LIST_HEAD(dvfs_info_list);
+
+static struct mtk_cpu_dvfs_info *mtk_cpu_dvfs_info_lookup(int cpu)
+{
+	struct mtk_cpu_dvfs_info *info;
+	struct list_head *list;
+
+	list_for_each(list, &dvfs_info_list) {
+		info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+		if (cpumask_test_cpu(cpu, &info->cpus))
+			return info;
+	}
+
+	return NULL;
+}
+
 static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 					int new_vproc)
 {

@@ -402,6 +421,9 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	 */
 	info->need_voltage_tracking = !IS_ERR(sram_reg);
 
+	/* CPUs in the same cluster share a clock and power domain. */
+	cpumask_copy(&info->cpus, &cpu_topology[cpu].core_sibling);
+
 	return 0;
 
 out_free_opp_table:

@@ -440,22 +462,18 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
 	struct cpufreq_frequency_table *freq_table;
 	int ret;
 
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
-
-	ret = mtk_cpu_dvfs_info_init(info, policy->cpu);
-	if (ret) {
-		pr_err("%s failed to initialize dvfs info for cpu%d\n",
-		       __func__, policy->cpu);
-		goto out_free_dvfs_info;
+	info = mtk_cpu_dvfs_info_lookup(policy->cpu);
+	if (!info) {
+		pr_err("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);
-		goto out_release_dvfs_info;
+		return ret;
 	}
 
 	ret = cpufreq_table_validate_and_show(policy, freq_table);

@@ -464,8 +482,7 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
 		goto out_free_cpufreq_table;
 	}
 
-	/* CPUs in the same cluster share a clock and power domain. */
-	cpumask_copy(policy->cpus, &cpu_topology[policy->cpu].core_sibling);
+	cpumask_copy(policy->cpus, &info->cpus);
 	policy->driver_data = info;
 	policy->clk = info->cpu_clk;
 

@@ -473,13 +490,6 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
 
 out_free_cpufreq_table:
 	dev_pm_opp_free_cpufreq_table(info->cpu_dev, &freq_table);
-
-out_release_dvfs_info:
-	mtk_cpu_dvfs_info_release(info);
-
-out_free_dvfs_info:
-	kfree(info);
-
 	return ret;
 }
 

@@ -489,8 +499,6 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
 
 	cpufreq_cooling_unregister(info->cdev);
 	dev_pm_opp_free_cpufreq_table(info->cpu_dev, &policy->freq_table);
-	mtk_cpu_dvfs_info_release(info);
-	kfree(info);
 
 	return 0;
 }

@@ -510,11 +518,47 @@ static struct cpufreq_driver mt8173_cpufreq_driver = {
 
 static int mt8173_cpufreq_probe(struct platform_device *pdev)
 {
-	int ret;
+	struct mtk_cpu_dvfs_info *info;
+	struct list_head *list, *tmp;
+	int cpu, ret;
+
+	for_each_possible_cpu(cpu) {
+		info = mtk_cpu_dvfs_info_lookup(cpu);
+		if (info)
+			continue;
+
+		info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+		if (!info) {
+			ret = -ENOMEM;
+			goto release_dvfs_info_list;
+		}
+
+		ret = mtk_cpu_dvfs_info_init(info, cpu);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"failed to initialize dvfs info for cpu%d\n",
+				cpu);
+			goto release_dvfs_info_list;
+		}
+
+		list_add(&info->list_head, &dvfs_info_list);
+	}
 
 	ret = cpufreq_register_driver(&mt8173_cpufreq_driver);
-	if (ret)
-		pr_err("failed to register mtk cpufreq driver\n");
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register mtk cpufreq driver\n");
+		goto release_dvfs_info_list;
+	}
+
+	return 0;
+
+release_dvfs_info_list:
+	list_for_each_safe(list, tmp, &dvfs_info_list) {
+		info = list_entry(list, struct mtk_cpu_dvfs_info, list_head);
+
+		mtk_cpu_dvfs_info_release(info);
+		list_del(list);
+	}
 
 	return ret;
 }
Signature · 1 line
1.9.1
#13 lore raw

PATCH v3 1/1 cpufreq: mt8173: move resources allocation into ->probe()

Quoted 6 lines · Pi-Cheng Chen

Since the return value of ->init() of cpufreq driver is not propagated to the device driver model now, move resources allocation into ->probe() to handle -EPROBE_DEFER properly.

Signed-off-by Pi-Cheng Chen <[email protected]>
Acked-by Viresh Kumar <[email protected]>

Applied along with the [1-3] in this series.

Thanks,
Rafael

maintainer
#14 lore raw

PATCH 1/4 cpufreq: mt8173: add CPUFREQ_HAVE_GOVERNOR_PER_POLICY flag

In reply to Pi-Cheng Chen
Quoted 13 lines · Pi-Cheng Chen

Add CPUFREQ_HAVE_GOVERNOR_PER_POLICY to have individual set of tunables for each cluster of MT8173.

Signed-off-by Pi-Cheng Chen <[email protected]>
---
 drivers/cpufreq/mt8173-cpufreq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/cpufreq/mt8173-cpufreq.c +0 -0
index 83001dc..c438109 100644

drivers/cpufreq/mt8173-cpufreq.c static int mtk_cpufreq_exit(struct cpufreq_policy *policy) · line 476

Quoted 9 lines
drivers/cpufreq/mt8173-cpufreq.c +2 -1
@@ -469,7 +469,8 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
 }
 
 static struct cpufreq_driver mt8173_cpufreq_driver = {
-	.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+	.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+		 CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
 	.verify = cpufreq_generic_frequency_table_verify,
 	.target_index = mtk_cpufreq_set_target,
 	.get = cpufreq_generic_get,
Acked-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh