[PATCH 1/2] PM / OPP: Remove OF dependency on dev_pm_opp_of_{cpumask_,}remove_table

[PATCH 1/2] PM / OPP: Remove OF dependency on dev_pm_opp_of_{cpumask_,}remove_table

[Sudeep Holla]

Functions dev_pm_opp_of_{cpumask_,}remove_table removes/frees all the
static OPP entries associated with the device and/or all cpus(in case
of cpumask). It makes no references to the device node in the DT. It
can be used even for usecases where the static OPP entries are populated
reading from the firmware or some different method.

This patch renames both dev_pm_opp_of_{cpumask_,}remove_table to
dev_pm_opp_{cpumask_,}remove_table so that it can be used in the above
mentioned usecase.

This is in preparation to make use of the same in scpi-cpufreq.c

Cc: Viresh Kumar <vireshk@kernel.org>
Cc: Nishanth Menon <nm@ti.com>
CC: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/base/power/opp/core.c        | 16 ++++++++--------
 drivers/base/power/opp/cpu.c         | 10 +++++-----
 drivers/cpufreq/arm_big_little_dt.c  |  2 +-
 drivers/cpufreq/cpufreq-dt.c         |  4 ++--
 drivers/cpufreq/exynos5440-cpufreq.c |  4 ++--
 drivers/cpufreq/imx6q-cpufreq.c      |  4 ++--
 drivers/cpufreq/mt8173-cpufreq.c     |  4 ++--
 include/linux/pm_opp.h               | 20 ++++++++++----------
 8 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 433b60092972..6716e02ad5ed 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1054,7 +1054,7 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
  * dev_pm_opp_enable/disable functions and may be removed by dev_pm_opp_remove.
  *
  * NOTE: "dynamic" parameter impacts OPPs added by the dev_pm_opp_of_add_table
- * and freed by dev_pm_opp_of_remove_table.
+ * and freed by dev_pm_opp_remove_table.
  *
  * Locking: The internal opp_table and opp structures are RCU protected.
  * Hence this function internally uses RCU updater strategy with mutex locks
@@ -1845,13 +1845,12 @@ struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
 
-#ifdef CONFIG_OF
 /**
- * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
- *				  entries
+ * dev_pm_opp_remove_table() - Free OPP table static entries associated with
+ *			       the device
  * @dev:	device pointer used to lookup OPP table.
  *
- * Free OPPs created using static entries present in DT.
+ * Free static OPP entries associated with the device.
  *
  * Locking: The internal opp_table and opp structures are RCU protected.
  * Hence this function indirectly uses RCU updater strategy with mutex locks
@@ -1859,7 +1858,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
  * that this function is *NOT* called under RCU protection or in contexts where
  * mutex cannot be locked.
  */
-void dev_pm_opp_of_remove_table(struct device *dev)
+void dev_pm_opp_remove_table(struct device *dev)
 {
 	struct opp_table *opp_table;
 	struct dev_pm_opp *opp, *tmp;
@@ -1894,8 +1893,9 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 unlock:
 	mutex_unlock(&opp_table_lock);
 }
-EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
+EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table);
 
+#ifdef CONFIG_OF
 /* Returns opp descriptor node for a device, caller must do of_node_put() */
 struct device_node *_of_get_opp_desc_node(struct device *dev)
 {
@@ -1961,7 +1961,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 	return 0;
 
 free_table:
-	dev_pm_opp_of_remove_table(dev);
+	dev_pm_opp_remove_table(dev);
 
 	return ret;
 }
diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
index 55cbf9bd8707..278bab29f76c 100644
--- a/drivers/base/power/opp/cpu.c
+++ b/drivers/base/power/opp/cpu.c
@@ -119,7 +119,6 @@ void dev_pm_opp_free_cpufreq_table(struct device *dev,
 EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
 #endif	/* CONFIG_CPU_FREQ */
 
-#ifdef CONFIG_OF
 /**
  * dev_pm_opp_of_cpumask_remove_table() - Removes OPP table for @cpumask
  * @cpumask:	cpumask for which OPP table needs to be removed
@@ -132,7 +131,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
  * that this function is *NOT* called under RCU protection or in contexts where
  * mutex cannot be locked.
  */
-void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
+void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
 {
 	struct device *cpu_dev;
 	int cpu;
@@ -147,11 +146,12 @@ void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
 			continue;
 		}
 
-		dev_pm_opp_of_remove_table(cpu_dev);
+		dev_pm_opp_remove_table(cpu_dev);
 	}
 }
-EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_remove_table);
+EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_remove_table);
 
+#ifdef CONFIG_OF
 /**
  * dev_pm_opp_of_cpumask_add_table() - Adds OPP table for @cpumask
  * @cpumask:	cpumask for which OPP table needs to be added.
@@ -185,7 +185,7 @@ int dev_pm_opp_of_cpumask_add_table(cpumask_var_t cpumask)
 			       __func__, cpu, ret);
 
 			/* Free all other OPPs */
-			dev_pm_opp_of_cpumask_remove_table(cpumask);
+			dev_pm_opp_cpumask_remove_table(cpumask);
 			break;
 		}
 	}
diff --git a/drivers/cpufreq/arm_big_little_dt.c b/drivers/cpufreq/arm_big_little_dt.c
index 16ddeefe9443..a8ff5e590125 100644
--- a/drivers/cpufreq/arm_big_little_dt.c
+++ b/drivers/cpufreq/arm_big_little_dt.c
@@ -82,7 +82,7 @@ static struct cpufreq_arm_bL_ops dt_bL_ops = {
 	.name	= "dt-bl",
 	.get_transition_latency = dt_get_transition_latency,
 	.init_opp_table = dt_init_opp_table,
-	.free_opp_table = dev_pm_opp_of_remove_table,
+	.free_opp_table = dev_pm_opp_remove_table,
 };
 
 static int generic_bL_probe(struct platform_device *pdev)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 5f8dbe640a20..c80681278f10 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -283,7 +283,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 out_free_priv:
 	kfree(priv);
 out_free_opp:
-	dev_pm_opp_of_cpumask_remove_table(policy->cpus);
+	dev_pm_opp_cpumask_remove_table(policy->cpus);
 	if (name)
 		dev_pm_opp_put_regulator(cpu_dev);
 out_put_clk:
@@ -298,7 +298,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
 
 	cpufreq_cooling_unregister(priv->cdev);
 	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
-	dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
+	dev_pm_opp_cpumask_remove_table(policy->related_cpus);
 	if (priv->reg_name)
 		dev_pm_opp_put_regulator(priv->cpu_dev);
 
diff --git a/drivers/cpufreq/exynos5440-cpufreq.c b/drivers/cpufreq/exynos5440-cpufreq.c
index c0f3373706f4..af5468c8e829 100644
--- a/drivers/cpufreq/exynos5440-cpufreq.c
+++ b/drivers/cpufreq/exynos5440-cpufreq.c
@@ -424,7 +424,7 @@ static int exynos_cpufreq_probe(struct platform_device *pdev)
 err_free_table:
 	dev_pm_opp_free_cpufreq_table(dvfs_info->dev, &dvfs_info->freq_table);
 err_free_opp:
-	dev_pm_opp_of_remove_table(dvfs_info->dev);
+	dev_pm_opp_remove_table(dvfs_info->dev);
 err_put_node:
 	of_node_put(np);
 	dev_err(&pdev->dev, "%s: failed initialization\n", __func__);
@@ -435,7 +435,7 @@ static int exynos_cpufreq_remove(struct platform_device *pdev)
 {
 	cpufreq_unregister_driver(&exynos_driver);
 	dev_pm_opp_free_cpufreq_table(dvfs_info->dev, &dvfs_info->freq_table);
-	dev_pm_opp_of_remove_table(dvfs_info->dev);
+	dev_pm_opp_remove_table(dvfs_info->dev);
 	return 0;
 }
 
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index ef1fa8145419..2bf5bf812f1a 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -346,7 +346,7 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
 out_free_opp:
 	if (free_opp)
-		dev_pm_opp_of_remove_table(cpu_dev);
+		dev_pm_opp_remove_table(cpu_dev);
 put_reg:
 	if (!IS_ERR(arm_reg))
 		regulator_put(arm_reg);
@@ -378,7 +378,7 @@ static int imx6q_cpufreq_remove(struct platform_device *pdev)
 	cpufreq_unregister_driver(&imx6q_cpufreq_driver);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
 	if (free_opp)
-		dev_pm_opp_of_remove_table(cpu_dev);
+		dev_pm_opp_remove_table(cpu_dev);
 	regulator_put(arm_reg);
 	if (!IS_ERR(pu_reg))
 		regulator_put(pu_reg);
diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mt8173-cpufreq.c
index 6f602c7a71bd..00fda9377d6e 100644
--- a/drivers/cpufreq/mt8173-cpufreq.c
+++ b/drivers/cpufreq/mt8173-cpufreq.c
@@ -430,7 +430,7 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	return 0;
 
 out_free_opp_table:
-	dev_pm_opp_of_cpumask_remove_table(&info->cpus);
+	dev_pm_opp_cpumask_remove_table(&info->cpus);
 
 out_free_resources:
 	if (!IS_ERR(proc_reg))
@@ -456,7 +456,7 @@ static void mtk_cpu_dvfs_info_release(struct mtk_cpu_dvfs_info *info)
 	if (!IS_ERR(info->inter_clk))
 		clk_put(info->inter_clk);
 
-	dev_pm_opp_of_cpumask_remove_table(&info->cpus);
+	dev_pm_opp_cpumask_remove_table(&info->cpus);
 }
 
 static int mtk_cpufreq_init(struct cpufreq_policy *policy)
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5b6ad31403a5..1907e89b7362 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -66,6 +66,8 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name);
 void dev_pm_opp_put_regulator(struct device *dev);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask);
+void dev_pm_opp_remove_table(struct device *dev);
+void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask);
 #else
 static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 {
@@ -184,13 +186,19 @@ static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_va
 	return -ENOSYS;
 }
 
+static inline void dev_pm_opp_remove_table(struct device *dev)
+{
+}
+
+static inline void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
+{
+}
+
 #endif		/* CONFIG_PM_OPP */
 
 #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
 int dev_pm_opp_of_add_table(struct device *dev);
-void dev_pm_opp_of_remove_table(struct device *dev);
 int dev_pm_opp_of_cpumask_add_table(cpumask_var_t cpumask);
-void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask);
 int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask);
 #else
 static inline int dev_pm_opp_of_add_table(struct device *dev)
@@ -198,19 +206,11 @@ static inline int dev_pm_opp_of_add_table(struct device *dev)
 	return -EINVAL;
 }
 
-static inline void dev_pm_opp_of_remove_table(struct device *dev)
-{
-}
-
 static inline int dev_pm_opp_of_cpumask_add_table(cpumask_var_t cpumask)
 {
 	return -ENOSYS;
 }
 
-static inline void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
-{
-}
-
 static inline int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
 {
 	return -ENOSYS;
-- 
1.9.1

[PATCH 2/2] cpufreq: arm_big_little: use generic OPP functions for {init,free}_opp_table

[Sudeep Holla]

Currently when performing random hotplugs and suspend-to-ram(S2R) on
systems using arm_big_little cpufreq driver, we get warnings similar to:

cpu cpu1: _opp_add: duplicate OPPs detected. Existing: freq: 600000000,
	volt: 800000, enabled: 1. New: freq: 600000000, volt: 800000, enabled: 1

This is mainly because the OPPs for the shared cpus are not set. We can
just use dev_pm_opp_of_cpumask_add_table in case the OPPs are obtained
from DT(arm_big_little_dt.c) or use dev_pm_opp_set_sharing_cpus if the
OPPs are obtained by other means like firmware(e.g. scpi-cpufreq.c)

Also now that the generic dev_pm_opp_cpumask_remove_table can handle
removal of opp table and entries for all associated CPUs, we can reuse
dev_pm_opp_cpumask_remove_table as free_opp_table in cpufreq_arm_bL_ops.

This patch makes necessary changes to reuse the generic OPP functions for
{init,free}_opp_table and thereby eliminating the warnings.

Cc: Viresh Kumar <vireshk@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpufreq/arm_big_little.c    | 54 ++++++++++++++++++++-----------------
 drivers/cpufreq/arm_big_little.h    |  4 +--
 drivers/cpufreq/arm_big_little_dt.c | 21 ++-------------
 drivers/cpufreq/scpi-cpufreq.c      | 24 ++++++++++++++---
 4 files changed, 54 insertions(+), 49 deletions(-)

Hi Viresh,

Why is dynamic OPPs not deleted in dev_pm_opp_{,cpumask_}remove_table ?
It would remove some code in scpi-cpufreq.c but I would like to understand.
Is there any use in not deleting them there ?

Regards,
Sudeep

diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index c251247ae661..6f3a951da31f 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -298,7 +298,8 @@ static int merge_cluster_tables(void)
 	return 0;
 }
 
-static void _put_cluster_clk_and_freq_table(struct device *cpu_dev)
+static void _put_cluster_clk_and_freq_table(struct device *cpu_dev,
+					    cpumask_var_t cpumask)
 {
 	u32 cluster = raw_cpu_to_cluster(cpu_dev->id);
 
@@ -308,11 +309,12 @@ static void _put_cluster_clk_and_freq_table(struct device *cpu_dev)
 	clk_put(clk[cluster]);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
 	if (arm_bL_ops->free_opp_table)
-		arm_bL_ops->free_opp_table(cpu_dev);
+		arm_bL_ops->free_opp_table(cpumask);
 	dev_dbg(cpu_dev, "%s: cluster: %d\n", __func__, cluster);
 }
 
-static void put_cluster_clk_and_freq_table(struct device *cpu_dev)
+static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
+					   cpumask_var_t cpumask)
 {
 	u32 cluster = cpu_to_cluster(cpu_dev->id);
 	int i;
@@ -321,7 +323,7 @@ static void put_cluster_clk_and_freq_table(struct device *cpu_dev)
 		return;
 
 	if (cluster < MAX_CLUSTERS)
-		return _put_cluster_clk_and_freq_table(cpu_dev);
+		return _put_cluster_clk_and_freq_table(cpu_dev, cpumask);
 
 	for_each_present_cpu(i) {
 		struct device *cdev = get_cpu_device(i);
@@ -330,14 +332,15 @@ static void put_cluster_clk_and_freq_table(struct device *cpu_dev)
 			return;
 		}
 
-		_put_cluster_clk_and_freq_table(cdev);
+		_put_cluster_clk_and_freq_table(cdev, cpumask);
 	}
 
 	/* free virtual table */
 	kfree(freq_table[cluster]);
 }
 
-static int _get_cluster_clk_and_freq_table(struct device *cpu_dev)
+static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
+					   cpumask_var_t cpumask)
 {
 	u32 cluster = raw_cpu_to_cluster(cpu_dev->id);
 	int ret;
@@ -345,7 +348,7 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev)
 	if (freq_table[cluster])
 		return 0;
 
-	ret = arm_bL_ops->init_opp_table(cpu_dev);
+	ret = arm_bL_ops->init_opp_table(cpumask);
 	if (ret) {
 		dev_err(cpu_dev, "%s: init_opp_table failed, cpu: %d, err: %d\n",
 				__func__, cpu_dev->id, ret);
@@ -374,14 +377,15 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev)
 
 free_opp_table:
 	if (arm_bL_ops->free_opp_table)
-		arm_bL_ops->free_opp_table(cpu_dev);
+		arm_bL_ops->free_opp_table(cpumask);
 out:
 	dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,
 			cluster);
 	return ret;
 }
 
-static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
+static int get_cluster_clk_and_freq_table(struct device *cpu_dev,
+					  cpumask_var_t cpumask)
 {
 	u32 cluster = cpu_to_cluster(cpu_dev->id);
 	int i, ret;
@@ -390,7 +394,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
 		return 0;
 
 	if (cluster < MAX_CLUSTERS) {
-		ret = _get_cluster_clk_and_freq_table(cpu_dev);
+		ret = _get_cluster_clk_and_freq_table(cpu_dev, cpumask);
 		if (ret)
 			atomic_dec(&cluster_usage[cluster]);
 		return ret;
@@ -407,7 +411,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
 			return -ENODEV;
 		}
 
-		ret = _get_cluster_clk_and_freq_table(cdev);
+		ret = _get_cluster_clk_and_freq_table(cdev, cpumask);
 		if (ret)
 			goto put_clusters;
 	}
@@ -433,7 +437,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
 			return -ENODEV;
 		}
 
-		_put_cluster_clk_and_freq_table(cdev);
+		_put_cluster_clk_and_freq_table(cdev, cpumask);
 	}
 
 	atomic_dec(&cluster_usage[cluster]);
@@ -455,18 +459,6 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 		return -ENODEV;
 	}
 
-	ret = get_cluster_clk_and_freq_table(cpu_dev);
-	if (ret)
-		return ret;
-
-	ret = cpufreq_table_validate_and_show(policy, freq_table[cur_cluster]);
-	if (ret) {
-		dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
-				policy->cpu, cur_cluster);
-		put_cluster_clk_and_freq_table(cpu_dev);
-		return ret;
-	}
-
 	if (cur_cluster < MAX_CLUSTERS) {
 		int cpu;
 
@@ -479,6 +471,18 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 		per_cpu(physical_cluster, policy->cpu) = A15_CLUSTER;
 	}
 
+	ret = get_cluster_clk_and_freq_table(cpu_dev, policy->cpus);
+	if (ret)
+		return ret;
+
+	ret = cpufreq_table_validate_and_show(policy, freq_table[cur_cluster]);
+	if (ret) {
+		dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
+			policy->cpu, cur_cluster);
+		put_cluster_clk_and_freq_table(cpu_dev, policy->cpus);
+		return ret;
+	}
+
 	if (arm_bL_ops->get_transition_latency)
 		policy->cpuinfo.transition_latency =
 			arm_bL_ops->get_transition_latency(cpu_dev);
@@ -509,7 +513,7 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 		return -ENODEV;
 	}
 
-	put_cluster_clk_and_freq_table(cpu_dev);
+	put_cluster_clk_and_freq_table(cpu_dev, policy->related_cpus);
 	dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
 
 	return 0;
diff --git a/drivers/cpufreq/arm_big_little.h b/drivers/cpufreq/arm_big_little.h
index b88889d9387e..adb32c36903b 100644
--- a/drivers/cpufreq/arm_big_little.h
+++ b/drivers/cpufreq/arm_big_little.h
@@ -30,11 +30,11 @@ struct cpufreq_arm_bL_ops {
 	 * This must set opp table for cpu_dev in a similar way as done by
 	 * dev_pm_opp_of_add_table().
 	 */
-	int (*init_opp_table)(struct device *cpu_dev);
+	int (*init_opp_table)(cpumask_var_t cpumask);
 
 	/* Optional */
 	int (*get_transition_latency)(struct device *cpu_dev);
-	void (*free_opp_table)(struct device *cpu_dev);
+	void (*free_opp_table)(cpumask_var_t cpumask);
 };
 
 int bL_cpufreq_register(struct cpufreq_arm_bL_ops *ops);
diff --git a/drivers/cpufreq/arm_big_little_dt.c b/drivers/cpufreq/arm_big_little_dt.c
index a8ff5e590125..8a66154a0cfd 100644
--- a/drivers/cpufreq/arm_big_little_dt.c
+++ b/drivers/cpufreq/arm_big_little_dt.c
@@ -43,23 +43,6 @@ static struct device_node *get_cpu_node_with_valid_op(int cpu)
 	return np;
 }
 
-static int dt_init_opp_table(struct device *cpu_dev)
-{
-	struct device_node *np;
-	int ret;
-
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_err("failed to find cpu%d node\n", cpu_dev->id);
-		return -ENOENT;
-	}
-
-	ret = dev_pm_opp_of_add_table(cpu_dev);
-	of_node_put(np);
-
-	return ret;
-}
-
 static int dt_get_transition_latency(struct device *cpu_dev)
 {
 	struct device_node *np;
@@ -81,8 +64,8 @@ static int dt_get_transition_latency(struct device *cpu_dev)
 static struct cpufreq_arm_bL_ops dt_bL_ops = {
 	.name	= "dt-bl",
 	.get_transition_latency = dt_get_transition_latency,
-	.init_opp_table = dt_init_opp_table,
-	.free_opp_table = dev_pm_opp_remove_table,
+	.init_opp_table = dev_pm_opp_of_cpumask_add_table,
+	.free_opp_table = dev_pm_opp_cpumask_remove_table,
 };
 
 static int generic_bL_probe(struct platform_device *pdev)
diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
index de5e89b2eaaa..83bd6050f227 100644
--- a/drivers/cpufreq/scpi-cpufreq.c
+++ b/drivers/cpufreq/scpi-cpufreq.c
@@ -18,6 +18,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -76,13 +77,30 @@ static int scpi_get_transition_latency(struct device *cpu_dev)
 	return info->latency;
 }
 
-static int scpi_init_opp_table(struct device *cpu_dev)
+static int scpi_init_opp_table(cpumask_var_t cpumask)
 {
-	return scpi_opp_table_ops(cpu_dev, false);
+	int ret;
+	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
+
+	ret = scpi_opp_table_ops(cpu_dev, false);
+	if (ret)
+		return ret;
+
+	ret = dev_pm_opp_set_sharing_cpus(cpu_dev, cpumask);
+	if (ret)
+		dev_err(cpu_dev, "%s: failed to mark OPPs as shared: %d\n",
+			__func__, ret);
+	return ret;
 }
 
-static void scpi_free_opp_table(struct device *cpu_dev)
+static void scpi_free_opp_table(cpumask_var_t cpumask)
 {
+	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
+
+	cpumask_clear_cpu(cpu_dev->id, cpumask);
+	dev_pm_opp_cpumask_remove_table(cpumask);
+	cpumask_set_cpu(cpu_dev->id, cpumask);
+
 	scpi_opp_table_ops(cpu_dev, true);
 }
 
-- 
1.9.1

Re: [PATCH 1/2] PM / OPP: Remove OF dependency on dev_pm_opp_of_{cpumask_,}remove_table

[Viresh Kumar]

On 28-04-16, 11:25, Sudeep Holla wrote:
> + * dev_pm_opp_remove_table() - Free OPP table static entries associated with
> + *			       the device

Its not about static entries anymore, right? We will end up removing everything
we had in the table.

Can you please update comments also in the same patch ?

-- 
viresh

Re: [PATCH 1/2] PM / OPP: Remove OF dependency on dev_pm_opp_of_{cpumask_,}remove_table

[Viresh Kumar]

On 28-04-16, 16:42, Viresh Kumar wrote:
> On 28-04-16, 11:25, Sudeep Holla wrote:
> > + * dev_pm_opp_remove_table() - Free OPP table static entries associated with
> > + *			       the device
> 
> Its not about static entries anymore, right? We will end up removing everything
> we had in the table.

Pardon me, that's not true..

-- 
viresh

Re: [PATCH 1/2] PM / OPP: Remove OF dependency on dev_pm_opp_of_{cpumask_,}remove_table

[Sudeep Holla]

On 28/04/16 12:12, Viresh Kumar wrote:
> On 28-04-16, 11:25, Sudeep Holla wrote:
>> + * dev_pm_opp_remove_table() - Free OPP table static entries associated with
>> + *			       the device
>
> Its not about static entries anymore, right? We will end up removing everything
> we had in the table.
>

No, not yet. I have not made that change yet. I just asked that question
in patch 2. I still remove individually but wanted to know if removing
dynamic opp is any issue ?

The OPP added using dev_pm_opp_add are marked dynamic and are not
deleted by dev_pm_opp_{,cpumask_}remove_table. If it does, then
scpi_free_opp_table can be assigned that instead of what I have in patch
2/2.

> Can you please update comments also in the same patch ?
>

Sure once you agree and I make that change  ;)

-- 
Regards,
Sudeep

Re: [PATCH 2/2] cpufreq: arm_big_little: use generic OPP functions for {init,free}_opp_table

[Viresh Kumar]

On 28-04-16, 11:25, Sudeep Holla wrote:
> Currently when performing random hotplugs and suspend-to-ram(S2R) on
> systems using arm_big_little cpufreq driver, we get warnings similar to:
> 
> cpu cpu1: _opp_add: duplicate OPPs detected. Existing: freq: 600000000,
> 	volt: 800000, enabled: 1. New: freq: 600000000, volt: 800000, enabled: 1
> 
> This is mainly because the OPPs for the shared cpus are not set. We can
> just use dev_pm_opp_of_cpumask_add_table in case the OPPs are obtained
> from DT(arm_big_little_dt.c) or use dev_pm_opp_set_sharing_cpus if the
> OPPs are obtained by other means like firmware(e.g. scpi-cpufreq.c)
> 
> Also now that the generic dev_pm_opp_cpumask_remove_table can handle
> removal of opp table and entries for all associated CPUs, we can reuse
> dev_pm_opp_cpumask_remove_table as free_opp_table in cpufreq_arm_bL_ops.
> 
> This patch makes necessary changes to reuse the generic OPP functions for
> {init,free}_opp_table and thereby eliminating the warnings.
> 
> Cc: Viresh Kumar <vireshk@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/cpufreq/arm_big_little.c    | 54 ++++++++++++++++++++-----------------
>  drivers/cpufreq/arm_big_little.h    |  4 +--
>  drivers/cpufreq/arm_big_little_dt.c | 21 ++-------------
>  drivers/cpufreq/scpi-cpufreq.c      | 24 ++++++++++++++---
>  4 files changed, 54 insertions(+), 49 deletions(-)
> 
> Hi Viresh,
> 
> Why is dynamic OPPs not deleted in dev_pm_opp_{,cpumask_}remove_table ?
> It would remove some code in scpi-cpufreq.c but I would like to understand.
> Is there any use in not deleting them there ?

That was intentional, consider this case:
- OPPs are added dynamically from platform code
- insmod cpufreq-dt.ko (add static (dt) OPPs as well, mostly fail)
- rmmod  cpufreq-dt.ko (remove all OPPs)

- insmod cpufreq-dt.ko (no more OPPs available, as we removed them both).

That's bad ?

Now, how to fix platforms which don't add dynamic OPPs from platform code? But
something like the scpi driver, which can get inserted/removed again.

This is what I would suggest:
- Even if dev_pm_opp_of_cpumask_remove_table() isn't doing any OF specific
  stuff, lets not update its name and move it out of the ifdef, as it will be
  used only if the user has created OPPs using DT earlier.
- But rather make those two routines call two new routines in the core (which
  don't depend on OF) and implement what these of-remove routines do.
- Add two more routines for removing OPPs created dynamically and call the two
  routines created in previous step.

I hope you followed that :)

> -static void scpi_free_opp_table(struct device *cpu_dev)
> +static void scpi_free_opp_table(cpumask_var_t cpumask)
>  {
> +	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
> +
> +	cpumask_clear_cpu(cpu_dev->id, cpumask);
> +	dev_pm_opp_cpumask_remove_table(cpumask);
> +	cpumask_set_cpu(cpu_dev->id, cpumask);
> +
>  	scpi_opp_table_ops(cpu_dev, true);
>  }

This was a *really* crazy idea :)

And btw, I think you better move to cpufreq-dt, instead of struggling with this
one :)

-- 
viresh

Re: [PATCH 2/2] cpufreq: arm_big_little: use generic OPP functions for {init,free}_opp_table

[Sudeep Holla]

On 28/04/16 12:26, Viresh Kumar wrote:
> On 28-04-16, 11:25, Sudeep Holla wrote:
>> Currently when performing random hotplugs and suspend-to-ram(S2R) on
>> systems using arm_big_little cpufreq driver, we get warnings similar to:
>>
>> cpu cpu1: _opp_add: duplicate OPPs detected. Existing: freq: 600000000,
>> 	volt: 800000, enabled: 1. New: freq: 600000000, volt: 800000, enabled: 1
>>
>> This is mainly because the OPPs for the shared cpus are not set. We can
>> just use dev_pm_opp_of_cpumask_add_table in case the OPPs are obtained
>> from DT(arm_big_little_dt.c) or use dev_pm_opp_set_sharing_cpus if the
>> OPPs are obtained by other means like firmware(e.g. scpi-cpufreq.c)
>>
>> Also now that the generic dev_pm_opp_cpumask_remove_table can handle
>> removal of opp table and entries for all associated CPUs, we can reuse
>> dev_pm_opp_cpumask_remove_table as free_opp_table in cpufreq_arm_bL_ops.
>>
>> This patch makes necessary changes to reuse the generic OPP functions for
>> {init,free}_opp_table and thereby eliminating the warnings.
>>
>> Cc: Viresh Kumar <vireshk@kernel.org>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: linux-pm@vger.kernel.org
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>   drivers/cpufreq/arm_big_little.c    | 54 ++++++++++++++++++++-----------------
>>   drivers/cpufreq/arm_big_little.h    |  4 +--
>>   drivers/cpufreq/arm_big_little_dt.c | 21 ++-------------
>>   drivers/cpufreq/scpi-cpufreq.c      | 24 ++++++++++++++---
>>   4 files changed, 54 insertions(+), 49 deletions(-)
>>
>> Hi Viresh,
>>
>> Why is dynamic OPPs not deleted in dev_pm_opp_{,cpumask_}remove_table ?
>> It would remove some code in scpi-cpufreq.c but I would like to understand.
>> Is there any use in not deleting them there ?
>
> That was intentional, consider this case:
> - OPPs are added dynamically from platform code
> - insmod cpufreq-dt.ko (add static (dt) OPPs as well, mostly fail)
> - rmmod  cpufreq-dt.ko (remove all OPPs)
>
> - insmod cpufreq-dt.ko (no more OPPs available, as we removed them both).
>
> That's bad ?
>
> Now, how to fix platforms which don't add dynamic OPPs from platform code? But
> something like the scpi driver, which can get inserted/removed again.
>
> This is what I would suggest:
> - Even if dev_pm_opp_of_cpumask_remove_table() isn't doing any OF specific
>    stuff, lets not update its name and move it out of the ifdef, as it will be
>    used only if the user has created OPPs using DT earlier.
> - But rather make those two routines call two new routines in the core (which
>    don't depend on OF) and implement what these of-remove routines do.
> - Add two more routines for removing OPPs created dynamically and call the two
>    routines created in previous step.
>
> I hope you followed that :)
>

Yes I got it. I had thought of this initially, but somehow got convinced 
that we can delete dynamic OPPs too.

>> -static void scpi_free_opp_table(struct device *cpu_dev)
>> +static void scpi_free_opp_table(cpumask_var_t cpumask)
>>   {
>> +	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
>> +
>> +	cpumask_clear_cpu(cpu_dev->id, cpumask);
>> +	dev_pm_opp_cpumask_remove_table(cpumask);
>> +	cpumask_set_cpu(cpu_dev->id, cpumask);
>> +
>>   	scpi_opp_table_ops(cpu_dev, true);
>>   }
>
> This was a *really* crazy idea :)
>

I knew that :)

> And btw, I think you better move to cpufreq-dt, instead of struggling with this
> one :)
>

Yes that's next on my TODO, but since this is duplicate messages are
getting reported now, it's better to fix that rather than wait for move
to cpufreq-dt. BTW cpufreq-dt will be misnomer after I make it work with
firmware driver DVFS.

Anyways, what ever change to fix these warning now will help to move to
cpufreq-dt IMO.

-- 
Regards,
Sudeep

[PATCH v2 1/2] PM / OPP: add non-OF versions of dev_pm_opp_{cpumask_,}remove_table

[Sudeep Holla]

Functions dev_pm_opp_of_{cpumask_,}remove_table removes/frees all the
static OPP entries associated with the device and/or all cpus(in case
of cpumask) that are created from DT.

However the OPP entries are populated reading from the firmware or some
different method using dev_pm_opp_add are marked dynamic and can't be
removed using above functions.

This patch adds non DT/OF versions of dev_pm_opp_{cpumask_,}remove_table
to support the above mentioned usecase.

This is in preparation to make use of the same in scpi-cpufreq.c

Cc: Viresh Kumar <vireshk@kernel.org>
Cc: Nishanth Menon <nm@ti.com>
CC: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/base/power/opp/core.c | 51 ++++++++++++++++++++++++++++++++-----
 drivers/base/power/opp/cpu.c  | 58 ++++++++++++++++++++++++++++++++-----------
 include/linux/pm_opp.h        | 10 ++++++++
 3 files changed, 98 insertions(+), 21 deletions(-)

v1->v2:
	- Instead of renaming OF versions, created non-OF versions of
	  dev_pm_opp_{cpumask_,}remove_table as suggested by Viresh

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 433b60092972..e59b9e7c31ba 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1845,13 +1845,14 @@ struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
 
-#ifdef CONFIG_OF
 /**
- * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
- *				  entries
+ * _dev_pm_opp_remove_table() - Free OPP table entries
  * @dev:	device pointer used to lookup OPP table.
+ * @remove_dyn:	specify whether to remove only OPPs created using
+ *              static entries from DT or even the dynamically add OPPs.
  *
- * Free OPPs created using static entries present in DT.
+ * Free OPPs either created using static entries present in DT or even the
+ * dynamically added entries based on @remove_dyn param.
  *
  * Locking: The internal opp_table and opp structures are RCU protected.
  * Hence this function indirectly uses RCU updater strategy with mutex locks
@@ -1859,7 +1860,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
  * that this function is *NOT* called under RCU protection or in contexts where
  * mutex cannot be locked.
  */
-void dev_pm_opp_of_remove_table(struct device *dev)
+static void _dev_pm_opp_remove_table(struct device *dev, bool remove_dyn)
 {
 	struct opp_table *opp_table;
 	struct dev_pm_opp *opp, *tmp;
@@ -1884,7 +1885,7 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 	if (list_is_singular(&opp_table->dev_list)) {
 		/* Free static OPPs */
 		list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
-			if (!opp->dynamic)
+			if (!opp->dynamic || (opp->dynamic && remove_dyn))
 				_opp_remove(opp_table, opp, true);
 		}
 	} else {
@@ -1894,6 +1895,44 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 unlock:
 	mutex_unlock(&opp_table_lock);
 }
+
+/**
+ * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
+ *				  entries
+ * @dev:	device pointer used to lookup OPP table.
+ *
+ * Free all OPPs associated with the device
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function indirectly uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_remove_table(struct device *dev)
+{
+	_dev_pm_opp_remove_table(dev, true);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table);
+
+#ifdef CONFIG_OF
+/**
+ * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
+ *				  entries
+ * @dev:	device pointer used to lookup OPP table.
+ *
+ * Free OPPs created using static entries present in DT.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function indirectly uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_of_remove_table(struct device *dev)
+{
+	_dev_pm_opp_remove_table(dev, false);
+}
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
 
 /* Returns opp descriptor node for a device, caller must do of_node_put() */
diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
index 55cbf9bd8707..9df4ad809c26 100644
--- a/drivers/base/power/opp/cpu.c
+++ b/drivers/base/power/opp/cpu.c
@@ -119,12 +119,54 @@ void dev_pm_opp_free_cpufreq_table(struct device *dev,
 EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
 #endif	/* CONFIG_CPU_FREQ */
 
+static void _dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask, bool of)
+{
+	struct device *cpu_dev;
+	int cpu;
+
+	WARN_ON(cpumask_empty(cpumask));
+
+	for_each_cpu(cpu, cpumask) {
+		cpu_dev = get_cpu_device(cpu);
+		if (!cpu_dev) {
+			pr_err("%s: failed to get cpu%d device\n", __func__,
+			       cpu);
+			continue;
+		}
+		if (of)
+			dev_pm_opp_of_remove_table(cpu_dev);
+		else
+			dev_pm_opp_remove_table(cpu_dev);
+	}
+}
+
+/**
+ * dev_pm_opp_of_cpumask_remove_table() - Removes OPP table for @cpumask
+ * @cpumask:	cpumask for which OPP table needs to be removed
+ *
+ * This removes the OPP tables for CPUs present in the @cpumask.
+ * This should be used to remove all the OPPs entries associated with
+ * the cpus in @cpumask.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
+{
+	_dev_pm_opp_cpumask_remove_table(cpumask, false);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_remove_table);
+
 #ifdef CONFIG_OF
 /**
  * dev_pm_opp_of_cpumask_remove_table() - Removes OPP table for @cpumask
  * @cpumask:	cpumask for which OPP table needs to be removed
  *
  * This removes the OPP tables for CPUs present in the @cpumask.
+ * This should be used only to remove static entries created from DT.
  *
  * Locking: The internal opp_table and opp structures are RCU protected.
  * Hence this function internally uses RCU updater strategy with mutex locks
@@ -134,21 +176,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
  */
 void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
 {
-	struct device *cpu_dev;
-	int cpu;
-
-	WARN_ON(cpumask_empty(cpumask));
-
-	for_each_cpu(cpu, cpumask) {
-		cpu_dev = get_cpu_device(cpu);
-		if (!cpu_dev) {
-			pr_err("%s: failed to get cpu%d device\n", __func__,
-			       cpu);
-			continue;
-		}
-
-		dev_pm_opp_of_remove_table(cpu_dev);
-	}
+	_dev_pm_opp_cpumask_remove_table(cpumask, true);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_remove_table);
 
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5b6ad31403a5..5c3781a79d31 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -66,6 +66,8 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name);
 void dev_pm_opp_put_regulator(struct device *dev);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask);
+void dev_pm_opp_remove_table(struct device *dev);
+void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask);
 #else
 static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 {
@@ -184,6 +186,14 @@ static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_va
 	return -ENOSYS;
 }
 
+static inline void dev_pm_opp_remove_table(struct device *dev)
+{
+}
+
+static inline void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
+{
+}
+
 #endif		/* CONFIG_PM_OPP */
 
 #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
-- 
1.9.1

[PATCH v2 2/2] cpufreq: arm_big_little: use generic OPP functions for {init,free}_opp_table

[Sudeep Holla]

Currently when performing random hotplugs and suspend-to-ram(S2R) on
systems using arm_big_little cpufreq driver, we get warnings similar to:

cpu cpu1: _opp_add: duplicate OPPs detected. Existing: freq: 600000000,
	volt: 800000, enabled: 1. New: freq: 600000000, volt: 800000, enabled: 1

This is mainly because the OPPs for the shared cpus are not set. We can
just use dev_pm_opp_of_cpumask_add_table in case the OPPs are obtained
from DT(arm_big_little_dt.c) or use dev_pm_opp_set_sharing_cpus if the
OPPs are obtained by other means like firmware(e.g. scpi-cpufreq.c)

Also now that the generic dev_pm_opp_cpumask_remove_table can handle
removal of opp table and entries for all associated CPUs, we can reuse
dev_pm_opp_cpumask_remove_table as free_opp_table in cpufreq_arm_bL_ops.

This patch makes necessary changes to reuse the generic OPP functions for
{init,free}_opp_table and thereby eliminating the warnings.

Cc: Viresh Kumar <vireshk@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpufreq/arm_big_little.c       | 54 ++++++++++++++++++----------------
 drivers/cpufreq/arm_big_little.h       |  4 +--
 drivers/cpufreq/arm_big_little_dt.c    | 21 ++-----------
 drivers/cpufreq/scpi-cpufreq.c         | 47 +++++++++++++----------------
 drivers/cpufreq/vexpress-spc-cpufreq.c |  4 ++-
 5 files changed, 56 insertions(+), 74 deletions(-)

diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index c251247ae661..6f3a951da31f 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -298,7 +298,8 @@ static int merge_cluster_tables(void)
 	return 0;
 }
 
-static void _put_cluster_clk_and_freq_table(struct device *cpu_dev)
+static void _put_cluster_clk_and_freq_table(struct device *cpu_dev,
+					    cpumask_var_t cpumask)
 {
 	u32 cluster = raw_cpu_to_cluster(cpu_dev->id);
 
@@ -308,11 +309,12 @@ static void _put_cluster_clk_and_freq_table(struct device *cpu_dev)
 	clk_put(clk[cluster]);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
 	if (arm_bL_ops->free_opp_table)
-		arm_bL_ops->free_opp_table(cpu_dev);
+		arm_bL_ops->free_opp_table(cpumask);
 	dev_dbg(cpu_dev, "%s: cluster: %d\n", __func__, cluster);
 }
 
-static void put_cluster_clk_and_freq_table(struct device *cpu_dev)
+static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
+					   cpumask_var_t cpumask)
 {
 	u32 cluster = cpu_to_cluster(cpu_dev->id);
 	int i;
@@ -321,7 +323,7 @@ static void put_cluster_clk_and_freq_table(struct device *cpu_dev)
 		return;
 
 	if (cluster < MAX_CLUSTERS)
-		return _put_cluster_clk_and_freq_table(cpu_dev);
+		return _put_cluster_clk_and_freq_table(cpu_dev, cpumask);
 
 	for_each_present_cpu(i) {
 		struct device *cdev = get_cpu_device(i);
@@ -330,14 +332,15 @@ static void put_cluster_clk_and_freq_table(struct device *cpu_dev)
 			return;
 		}
 
-		_put_cluster_clk_and_freq_table(cdev);
+		_put_cluster_clk_and_freq_table(cdev, cpumask);
 	}
 
 	/* free virtual table */
 	kfree(freq_table[cluster]);
 }
 
-static int _get_cluster_clk_and_freq_table(struct device *cpu_dev)
+static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
+					   cpumask_var_t cpumask)
 {
 	u32 cluster = raw_cpu_to_cluster(cpu_dev->id);
 	int ret;
@@ -345,7 +348,7 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev)
 	if (freq_table[cluster])
 		return 0;
 
-	ret = arm_bL_ops->init_opp_table(cpu_dev);
+	ret = arm_bL_ops->init_opp_table(cpumask);
 	if (ret) {
 		dev_err(cpu_dev, "%s: init_opp_table failed, cpu: %d, err: %d\n",
 				__func__, cpu_dev->id, ret);
@@ -374,14 +377,15 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev)
 
 free_opp_table:
 	if (arm_bL_ops->free_opp_table)
-		arm_bL_ops->free_opp_table(cpu_dev);
+		arm_bL_ops->free_opp_table(cpumask);
 out:
 	dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,
 			cluster);
 	return ret;
 }
 
-static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
+static int get_cluster_clk_and_freq_table(struct device *cpu_dev,
+					  cpumask_var_t cpumask)
 {
 	u32 cluster = cpu_to_cluster(cpu_dev->id);
 	int i, ret;
@@ -390,7 +394,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
 		return 0;
 
 	if (cluster < MAX_CLUSTERS) {
-		ret = _get_cluster_clk_and_freq_table(cpu_dev);
+		ret = _get_cluster_clk_and_freq_table(cpu_dev, cpumask);
 		if (ret)
 			atomic_dec(&cluster_usage[cluster]);
 		return ret;
@@ -407,7 +411,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
 			return -ENODEV;
 		}
 
-		ret = _get_cluster_clk_and_freq_table(cdev);
+		ret = _get_cluster_clk_and_freq_table(cdev, cpumask);
 		if (ret)
 			goto put_clusters;
 	}
@@ -433,7 +437,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
 			return -ENODEV;
 		}
 
-		_put_cluster_clk_and_freq_table(cdev);
+		_put_cluster_clk_and_freq_table(cdev, cpumask);
 	}
 
 	atomic_dec(&cluster_usage[cluster]);
@@ -455,18 +459,6 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 		return -ENODEV;
 	}
 
-	ret = get_cluster_clk_and_freq_table(cpu_dev);
-	if (ret)
-		return ret;
-
-	ret = cpufreq_table_validate_and_show(policy, freq_table[cur_cluster]);
-	if (ret) {
-		dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
-				policy->cpu, cur_cluster);
-		put_cluster_clk_and_freq_table(cpu_dev);
-		return ret;
-	}
-
 	if (cur_cluster < MAX_CLUSTERS) {
 		int cpu;
 
@@ -479,6 +471,18 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 		per_cpu(physical_cluster, policy->cpu) = A15_CLUSTER;
 	}
 
+	ret = get_cluster_clk_and_freq_table(cpu_dev, policy->cpus);
+	if (ret)
+		return ret;
+
+	ret = cpufreq_table_validate_and_show(policy, freq_table[cur_cluster]);
+	if (ret) {
+		dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
+			policy->cpu, cur_cluster);
+		put_cluster_clk_and_freq_table(cpu_dev, policy->cpus);
+		return ret;
+	}
+
 	if (arm_bL_ops->get_transition_latency)
 		policy->cpuinfo.transition_latency =
 			arm_bL_ops->get_transition_latency(cpu_dev);
@@ -509,7 +513,7 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 		return -ENODEV;
 	}
 
-	put_cluster_clk_and_freq_table(cpu_dev);
+	put_cluster_clk_and_freq_table(cpu_dev, policy->related_cpus);
 	dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
 
 	return 0;
diff --git a/drivers/cpufreq/arm_big_little.h b/drivers/cpufreq/arm_big_little.h
index b88889d9387e..adb32c36903b 100644
--- a/drivers/cpufreq/arm_big_little.h
+++ b/drivers/cpufreq/arm_big_little.h
@@ -30,11 +30,11 @@ struct cpufreq_arm_bL_ops {
 	 * This must set opp table for cpu_dev in a similar way as done by
 	 * dev_pm_opp_of_add_table().
 	 */
-	int (*init_opp_table)(struct device *cpu_dev);
+	int (*init_opp_table)(cpumask_var_t cpumask);
 
 	/* Optional */
 	int (*get_transition_latency)(struct device *cpu_dev);
-	void (*free_opp_table)(struct device *cpu_dev);
+	void (*free_opp_table)(cpumask_var_t cpumask);
 };
 
 int bL_cpufreq_register(struct cpufreq_arm_bL_ops *ops);
diff --git a/drivers/cpufreq/arm_big_little_dt.c b/drivers/cpufreq/arm_big_little_dt.c
index 16ddeefe9443..39b3f51d9a30 100644
--- a/drivers/cpufreq/arm_big_little_dt.c
+++ b/drivers/cpufreq/arm_big_little_dt.c
@@ -43,23 +43,6 @@ static struct device_node *get_cpu_node_with_valid_op(int cpu)
 	return np;
 }
 
-static int dt_init_opp_table(struct device *cpu_dev)
-{
-	struct device_node *np;
-	int ret;
-
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_err("failed to find cpu%d node\n", cpu_dev->id);
-		return -ENOENT;
-	}
-
-	ret = dev_pm_opp_of_add_table(cpu_dev);
-	of_node_put(np);
-
-	return ret;
-}
-
 static int dt_get_transition_latency(struct device *cpu_dev)
 {
 	struct device_node *np;
@@ -81,8 +64,8 @@ static int dt_get_transition_latency(struct device *cpu_dev)
 static struct cpufreq_arm_bL_ops dt_bL_ops = {
 	.name	= "dt-bl",
 	.get_transition_latency = dt_get_transition_latency,
-	.init_opp_table = dt_init_opp_table,
-	.free_opp_table = dev_pm_opp_of_remove_table,
+	.init_opp_table = dev_pm_opp_of_cpumask_add_table,
+	.free_opp_table = dev_pm_opp_of_cpumask_remove_table,
 };
 
 static int generic_bL_probe(struct platform_device *pdev)
diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
index de5e89b2eaaa..3fd54cf78b78 100644
--- a/drivers/cpufreq/scpi-cpufreq.c
+++ b/drivers/cpufreq/scpi-cpufreq.c
@@ -18,6 +18,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -38,10 +39,20 @@ static struct scpi_dvfs_info *scpi_get_dvfs_info(struct device *cpu_dev)
 	return scpi_ops->dvfs_get_info(domain);
 }
 
-static int scpi_opp_table_ops(struct device *cpu_dev, bool remove)
+static int scpi_get_transition_latency(struct device *cpu_dev)
 {
-	int idx, ret = 0;
+	struct scpi_dvfs_info *info = scpi_get_dvfs_info(cpu_dev);
+
+	if (IS_ERR(info))
+		return PTR_ERR(info);
+	return info->latency;
+}
+
+static int scpi_init_opp_table(cpumask_var_t cpumask)
+{
+	int idx, ret;
 	struct scpi_opp *opp;
+	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
 	struct scpi_dvfs_info *info = scpi_get_dvfs_info(cpu_dev);
 
 	if (IS_ERR(info))
@@ -51,11 +62,7 @@ static int scpi_opp_table_ops(struct device *cpu_dev, bool remove)
 		return -EIO;
 
 	for (opp = info->opps, idx = 0; idx < info->count; idx++, opp++) {
-		if (remove)
-			dev_pm_opp_remove(cpu_dev, opp->freq);
-		else
-			ret = dev_pm_opp_add(cpu_dev, opp->freq,
-					     opp->m_volt * 1000);
+		ret = dev_pm_opp_add(cpu_dev, opp->freq, opp->m_volt * 1000);
 		if (ret) {
 			dev_warn(cpu_dev, "failed to add opp %uHz %umV\n",
 				 opp->freq, opp->m_volt);
@@ -64,33 +71,19 @@ static int scpi_opp_table_ops(struct device *cpu_dev, bool remove)
 			return ret;
 		}
 	}
-	return ret;
-}
 
-static int scpi_get_transition_latency(struct device *cpu_dev)
-{
-	struct scpi_dvfs_info *info = scpi_get_dvfs_info(cpu_dev);
-
-	if (IS_ERR(info))
-		return PTR_ERR(info);
-	return info->latency;
-}
-
-static int scpi_init_opp_table(struct device *cpu_dev)
-{
-	return scpi_opp_table_ops(cpu_dev, false);
-}
-
-static void scpi_free_opp_table(struct device *cpu_dev)
-{
-	scpi_opp_table_ops(cpu_dev, true);
+	ret = dev_pm_opp_set_sharing_cpus(cpu_dev, cpumask);
+	if (ret)
+		dev_err(cpu_dev, "%s: failed to mark OPPs as shared: %d\n",
+			__func__, ret);
+	return ret;
 }
 
 static struct cpufreq_arm_bL_ops scpi_cpufreq_ops = {
 	.name	= "scpi",
 	.get_transition_latency = scpi_get_transition_latency,
 	.init_opp_table = scpi_init_opp_table,
-	.free_opp_table = scpi_free_opp_table,
+	.free_opp_table = dev_pm_opp_cpumask_remove_table,
 };
 
 static int scpi_cpufreq_probe(struct platform_device *pdev)
diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
index 433e93fd4900..5c1016541b91 100644
--- a/drivers/cpufreq/vexpress-spc-cpufreq.c
+++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
@@ -18,6 +18,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -26,8 +27,9 @@
 
 #include "arm_big_little.h"
 
-static int ve_spc_init_opp_table(struct device *cpu_dev)
+static int ve_spc_init_opp_table(cpumask_var_t cpumask)
 {
+	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
 	/*
 	 * platform specific SPC code must initialise the opp table
 	 * so just check if the OPP count is non-zero
-- 
1.9.1

Re: [PATCH v2 1/2] PM / OPP: add non-OF versions of dev_pm_opp_{cpumask_,}remove_table

[Viresh Kumar]

On 28-04-16, 18:07, Sudeep Holla wrote:
> diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
> index 433b60092972..e59b9e7c31ba 100644
> --- a/drivers/base/power/opp/core.c
> +++ b/drivers/base/power/opp/core.c
> @@ -1845,13 +1845,14 @@ struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
>  
> -#ifdef CONFIG_OF
>  /**
> - * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
> - *				  entries
> + * _dev_pm_opp_remove_table() - Free OPP table entries

This is an internal routine and doesn't really require a doc-style comment at
all. Please remove it. You can add a simple comment for things you want to
mention though.

>   * @dev:	device pointer used to lookup OPP table.
> + * @remove_dyn:	specify whether to remove only OPPs created using
> + *              static entries from DT or even the dynamically add OPPs.
>   *
> - * Free OPPs created using static entries present in DT.
> + * Free OPPs either created using static entries present in DT or even the
> + * dynamically added entries based on @remove_dyn param.
>   *
>   * Locking: The internal opp_table and opp structures are RCU protected.
>   * Hence this function indirectly uses RCU updater strategy with mutex locks
> @@ -1859,7 +1860,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
>   * that this function is *NOT* called under RCU protection or in contexts where
>   * mutex cannot be locked.
>   */
> -void dev_pm_opp_of_remove_table(struct device *dev)
> +static void _dev_pm_opp_remove_table(struct device *dev, bool remove_dyn)

Maybe s/remove_dyn/remove_all ..

>  {
>  	struct opp_table *opp_table;
>  	struct dev_pm_opp *opp, *tmp;
> @@ -1884,7 +1885,7 @@ void dev_pm_opp_of_remove_table(struct device *dev)
>  	if (list_is_singular(&opp_table->dev_list)) {
>  		/* Free static OPPs */
>  		list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
> -			if (!opp->dynamic)
> +			if (!opp->dynamic || (opp->dynamic && remove_dyn))

Well, that's a funny one :)

The second conditional statement doesn't require opp->dynamic, as that is
guaranteed to be true, as the first condition failed.

So this should be:

if (remove_all || !opp->dynamic)

>  				_opp_remove(opp_table, opp, true);
>  		}
>  	} else {
> @@ -1894,6 +1895,44 @@ void dev_pm_opp_of_remove_table(struct device *dev)
>  unlock:
>  	mutex_unlock(&opp_table_lock);
>  }
> +
> +/**
> + * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT

No, this isn't the OF specific function.

> + *				  entries
> + * @dev:	device pointer used to lookup OPP table.
> + *
> + * Free all OPPs associated with the device

Full stop at the end.

> + *
> + * Locking: The internal opp_table and opp structures are RCU protected.
> + * Hence this function indirectly uses RCU updater strategy with mutex locks
> + * to keep the integrity of the internal data structures. Callers should ensure
> + * that this function is *NOT* called under RCU protection or in contexts where
> + * mutex cannot be locked.
> + */
> +void dev_pm_opp_remove_table(struct device *dev)
> +{
> +	_dev_pm_opp_remove_table(dev, true);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table);
> +
> +#ifdef CONFIG_OF
> +/**
> + * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
> + *				  entries
> + * @dev:	device pointer used to lookup OPP table.
> + *
> + * Free OPPs created using static entries present in DT.
> + *
> + * Locking: The internal opp_table and opp structures are RCU protected.
> + * Hence this function indirectly uses RCU updater strategy with mutex locks
> + * to keep the integrity of the internal data structures. Callers should ensure
> + * that this function is *NOT* called under RCU protection or in contexts where
> + * mutex cannot be locked.
> + */
> +void dev_pm_opp_of_remove_table(struct device *dev)
> +{
> +	_dev_pm_opp_remove_table(dev, false);
> +}
>  EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
>  
>  /* Returns opp descriptor node for a device, caller must do of_node_put() */
> diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
> index 55cbf9bd8707..9df4ad809c26 100644
> --- a/drivers/base/power/opp/cpu.c
> +++ b/drivers/base/power/opp/cpu.c
> @@ -119,12 +119,54 @@ void dev_pm_opp_free_cpufreq_table(struct device *dev,
>  EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
>  #endif	/* CONFIG_CPU_FREQ */
>  
> +static void _dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask, bool of)
> +{
> +	struct device *cpu_dev;
> +	int cpu;
> +
> +	WARN_ON(cpumask_empty(cpumask));
> +
> +	for_each_cpu(cpu, cpumask) {
> +		cpu_dev = get_cpu_device(cpu);
> +		if (!cpu_dev) {
> +			pr_err("%s: failed to get cpu%d device\n", __func__,
> +			       cpu);
> +			continue;
> +		}

Blank line here.

> +		if (of)
> +			dev_pm_opp_of_remove_table(cpu_dev);
> +		else
> +			dev_pm_opp_remove_table(cpu_dev);
> +	}
> +}
> +
> +/**
> + * dev_pm_opp_of_cpumask_remove_table() - Removes OPP table for @cpumask

of :(

> + * @cpumask:	cpumask for which OPP table needs to be removed
> + *
> + * This removes the OPP tables for CPUs present in the @cpumask.
> + * This should be used to remove all the OPPs entries associated with
> + * the cpus in @cpumask.
> + *
> + * Locking: The internal opp_table and opp structures are RCU protected.
> + * Hence this function internally uses RCU updater strategy with mutex locks
> + * to keep the integrity of the internal data structures. Callers should ensure
> + * that this function is *NOT* called under RCU protection or in contexts where
> + * mutex cannot be locked.
> + */
> +void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
> +{
> +	_dev_pm_opp_cpumask_remove_table(cpumask, false);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_remove_table);
> +
>  #ifdef CONFIG_OF
>  /**
>   * dev_pm_opp_of_cpumask_remove_table() - Removes OPP table for @cpumask
>   * @cpumask:	cpumask for which OPP table needs to be removed
>   *
>   * This removes the OPP tables for CPUs present in the @cpumask.
> + * This should be used only to remove static entries created from DT.
>   *
>   * Locking: The internal opp_table and opp structures are RCU protected.
>   * Hence this function internally uses RCU updater strategy with mutex locks
> @@ -134,21 +176,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
>   */
>  void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
>  {
> -	struct device *cpu_dev;
> -	int cpu;
> -
> -	WARN_ON(cpumask_empty(cpumask));
> -
> -	for_each_cpu(cpu, cpumask) {
> -		cpu_dev = get_cpu_device(cpu);
> -		if (!cpu_dev) {
> -			pr_err("%s: failed to get cpu%d device\n", __func__,
> -			       cpu);
> -			continue;
> -		}
> -
> -		dev_pm_opp_of_remove_table(cpu_dev);
> -	}
> +	_dev_pm_opp_cpumask_remove_table(cpumask, true);
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_remove_table);
>  
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index 5b6ad31403a5..5c3781a79d31 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -66,6 +66,8 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name);
>  void dev_pm_opp_put_regulator(struct device *dev);
>  int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
>  int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask);
> +void dev_pm_opp_remove_table(struct device *dev);
> +void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask);
>  #else
>  static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
>  {
> @@ -184,6 +186,14 @@ static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_va
>  	return -ENOSYS;
>  }
>  
> +static inline void dev_pm_opp_remove_table(struct device *dev)
> +{
> +}
> +
> +static inline void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
> +{
> +}
> +
>  #endif		/* CONFIG_PM_OPP */
>  
>  #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
> -- 
> 1.9.1

-- 
viresh

Re: [PATCH v2 2/2] cpufreq: arm_big_little: use generic OPP functions for {init,free}_opp_table

[Viresh Kumar]

On 28-04-16, 18:07, Sudeep Holla wrote:
> Currently when performing random hotplugs and suspend-to-ram(S2R) on
> systems using arm_big_little cpufreq driver, we get warnings similar to:
> 
> cpu cpu1: _opp_add: duplicate OPPs detected. Existing: freq: 600000000,
> 	volt: 800000, enabled: 1. New: freq: 600000000, volt: 800000, enabled: 1
> 
> This is mainly because the OPPs for the shared cpus are not set. We can
> just use dev_pm_opp_of_cpumask_add_table in case the OPPs are obtained
> from DT(arm_big_little_dt.c) or use dev_pm_opp_set_sharing_cpus if the
> OPPs are obtained by other means like firmware(e.g. scpi-cpufreq.c)
> 
> Also now that the generic dev_pm_opp_cpumask_remove_table can handle
> removal of opp table and entries for all associated CPUs, we can reuse
> dev_pm_opp_cpumask_remove_table as free_opp_table in cpufreq_arm_bL_ops.
> 
> This patch makes necessary changes to reuse the generic OPP functions for
> {init,free}_opp_table and thereby eliminating the warnings.
> 
> Cc: Viresh Kumar <vireshk@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/cpufreq/arm_big_little.c       | 54 ++++++++++++++++++----------------
>  drivers/cpufreq/arm_big_little.h       |  4 +--
>  drivers/cpufreq/arm_big_little_dt.c    | 21 ++-----------
>  drivers/cpufreq/scpi-cpufreq.c         | 47 +++++++++++++----------------
>  drivers/cpufreq/vexpress-spc-cpufreq.c |  4 ++-
>  5 files changed, 56 insertions(+), 74 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 1/2] PM / OPP: add non-OF versions of dev_pm_opp_{cpumask_,}remove_table

[Sudeep Holla]

On 29/04/16 05:07, Viresh Kumar wrote:
> On 28-04-16, 18:07, Sudeep Holla wrote:
>> diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
>> index 433b60092972..e59b9e7c31ba 100644
>> --- a/drivers/base/power/opp/core.c
>> +++ b/drivers/base/power/opp/core.c

[...]

>>   {
>>   	struct opp_table *opp_table;
>>   	struct dev_pm_opp *opp, *tmp;
>> @@ -1884,7 +1885,7 @@ void dev_pm_opp_of_remove_table(struct device *dev)
>>   	if (list_is_singular(&opp_table->dev_list)) {
>>   		/* Free static OPPs */
>>   		list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
>> -			if (!opp->dynamic)
>> +			if (!opp->dynamic || (opp->dynamic && remove_dyn))
>
> Well, that's a funny one :)
>
> The second conditional statement doesn't require opp->dynamic, as that is
> guaranteed to be true, as the first condition failed.
>
> So this should be:
>
> if (remove_all || !opp->dynamic)
>

Yes, initially I thought we may need to support a mix where both static
entries from DT and dynamic entries will be present and this function
must remove the latter only. But I soon realized that it's not required
and also will get ugly. However I forgot to change this part :(

>>   				_opp_remove(opp_table, opp, true);
>>   		}
>>   	} else {
>> @@ -1894,6 +1895,44 @@ void dev_pm_opp_of_remove_table(struct device *dev)
>>   unlock:
>>   	mutex_unlock(&opp_table_lock);
>>   }
>> +
>> +/**
>> + * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
>
> No, this isn't the OF specific function.
>

Sorry, copy-paste disease :) esp. for documenting.

-- 
Regards,
Sudeep

[PATCH v2 1/2][UPDATE] PM / OPP: add non-OF versions of dev_pm_opp_{cpumask_,}remove_table

[Sudeep Holla]

Functions dev_pm_opp_of_{cpumask_,}remove_table removes/frees all the
static OPP entries associated with the device and/or all cpus(in case
of cpumask) that are created from DT.

However the OPP entries are populated reading from the firmware or some
different method using dev_pm_opp_add are marked dynamic and can't be
removed using above functions.

This patch adds non DT/OF versions of dev_pm_opp_{cpumask_,}remove_table
to support the above mentioned usecase.

This is in preparation to make use of the same in scpi-cpufreq.c

Cc: Viresh Kumar <vireshk@kernel.org>
Cc: Nishanth Menon <nm@ti.com>
CC: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/base/power/opp/core.c | 56 ++++++++++++++++++++++++++++++----------
 drivers/base/power/opp/cpu.c  | 59 ++++++++++++++++++++++++++++++++-----------
 include/linux/pm_opp.h        | 10 ++++++++
 3 files changed, 96 insertions(+), 29 deletions(-)

v1->v2:
	- Instead of renaming OF versions, created non-OF versions of
	  dev_pm_opp_{cpumask_,}remove_table as suggested by Viresh

This version also updates all the errors in documentation and changes
to use remove_all rather than remove_dyn.

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 433b60092972..15536d45832a 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1845,21 +1845,11 @@ struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
 
-#ifdef CONFIG_OF
 /**
- * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
- *				  entries
- * @dev:	device pointer used to lookup OPP table.
- *
- * Free OPPs created using static entries present in DT.
- *
- * Locking: The internal opp_table and opp structures are RCU protected.
- * Hence this function indirectly uses RCU updater strategy with mutex locks
- * to keep the integrity of the internal data structures. Callers should ensure
- * that this function is *NOT* called under RCU protection or in contexts where
- * mutex cannot be locked.
+ * Free OPPs either created using static entries present in DT or even the
+ * dynamically added entries based on remove_all param.
  */
-void dev_pm_opp_of_remove_table(struct device *dev)
+static void _dev_pm_opp_remove_table(struct device *dev, bool remove_all)
 {
 	struct opp_table *opp_table;
 	struct dev_pm_opp *opp, *tmp;
@@ -1884,7 +1874,7 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 	if (list_is_singular(&opp_table->dev_list)) {
 		/* Free static OPPs */
 		list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
-			if (!opp->dynamic)
+			if (remove_all || !opp->dynamic)
 				_opp_remove(opp_table, opp, true);
 		}
 	} else {
@@ -1894,6 +1884,44 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 unlock:
 	mutex_unlock(&opp_table_lock);
 }
+
+/**
+ * dev_pm_opp_remove_table() - Free all OPPs associated with the device
+ * @dev:	device pointer used to lookup OPP table.
+ *
+ * Free both OPPs created using static entries present in DT and the
+ * dynamically added entries.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function indirectly uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_remove_table(struct device *dev)
+{
+	_dev_pm_opp_remove_table(dev, true);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table);
+
+#ifdef CONFIG_OF
+/**
+ * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
+ *				  entries
+ * @dev:	device pointer used to lookup OPP table.
+ *
+ * Free OPPs created using static entries present in DT.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function indirectly uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_of_remove_table(struct device *dev)
+{
+	_dev_pm_opp_remove_table(dev, false);
+}
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
 
 /* Returns opp descriptor node for a device, caller must do of_node_put() */
diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
index 55cbf9bd8707..2dc39543d88b 100644
--- a/drivers/base/power/opp/cpu.c
+++ b/drivers/base/power/opp/cpu.c
@@ -119,20 +119,7 @@ void dev_pm_opp_free_cpufreq_table(struct device *dev,
 EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
 #endif	/* CONFIG_CPU_FREQ */
 
-#ifdef CONFIG_OF
-/**
- * dev_pm_opp_of_cpumask_remove_table() - Removes OPP table for @cpumask
- * @cpumask:	cpumask for which OPP table needs to be removed
- *
- * This removes the OPP tables for CPUs present in the @cpumask.
- *
- * Locking: The internal opp_table and opp structures are RCU protected.
- * Hence this function internally uses RCU updater strategy with mutex locks
- * to keep the integrity of the internal data structures. Callers should ensure
- * that this function is *NOT* called under RCU protection or in contexts where
- * mutex cannot be locked.
- */
-void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
+static void _dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask, bool of)
 {
 	struct device *cpu_dev;
 	int cpu;
@@ -147,9 +134,51 @@ void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
 			continue;
 		}
 
-		dev_pm_opp_of_remove_table(cpu_dev);
+		if (of)
+			dev_pm_opp_of_remove_table(cpu_dev);
+		else
+			dev_pm_opp_remove_table(cpu_dev);
 	}
 }
+
+/**
+ * dev_pm_opp_cpumask_remove_table() - Removes OPP table for @cpumask
+ * @cpumask:	cpumask for which OPP table needs to be removed
+ *
+ * This removes the OPP tables for CPUs present in the @cpumask.
+ * This should be used to remove all the OPPs entries associated with
+ * the cpus in @cpumask.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
+{
+	_dev_pm_opp_cpumask_remove_table(cpumask, false);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_remove_table);
+
+#ifdef CONFIG_OF
+/**
+ * dev_pm_opp_of_cpumask_remove_table() - Removes OPP table for @cpumask
+ * @cpumask:	cpumask for which OPP table needs to be removed
+ *
+ * This removes the OPP tables for CPUs present in the @cpumask.
+ * This should be used only to remove static entries created from DT.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_of_cpumask_remove_table(cpumask_var_t cpumask)
+{
+	_dev_pm_opp_cpumask_remove_table(cpumask, true);
+}
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_remove_table);
 
 /**
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5b6ad31403a5..5c3781a79d31 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -66,6 +66,8 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name);
 void dev_pm_opp_put_regulator(struct device *dev);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask);
+void dev_pm_opp_remove_table(struct device *dev);
+void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask);
 #else
 static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 {
@@ -184,6 +186,14 @@ static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_va
 	return -ENOSYS;
 }
 
+static inline void dev_pm_opp_remove_table(struct device *dev)
+{
+}
+
+static inline void dev_pm_opp_cpumask_remove_table(cpumask_var_t cpumask)
+{
+}
+
 #endif		/* CONFIG_PM_OPP */
 
 #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
-- 
1.9.1

Re: [PATCH v2 1/2][UPDATE] PM / OPP: add non-OF versions of dev_pm_opp_{cpumask_,}remove_table

[Viresh Kumar]

This isn't V2, but V3

On 29-04-16, 10:22, Sudeep Holla wrote:
> Functions dev_pm_opp_of_{cpumask_,}remove_table removes/frees all the
> static OPP entries associated with the device and/or all cpus(in case
> of cpumask) that are created from DT.
> 
> However the OPP entries are populated reading from the firmware or some
> different method using dev_pm_opp_add are marked dynamic and can't be
> removed using above functions.
> 
> This patch adds non DT/OF versions of dev_pm_opp_{cpumask_,}remove_table
> to support the above mentioned usecase.
> 
> This is in preparation to make use of the same in scpi-cpufreq.c
> 
> Cc: Viresh Kumar <vireshk@kernel.org>
> Cc: Nishanth Menon <nm@ti.com>
> CC: Stephen Boyd <sboyd@codeaurora.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/base/power/opp/core.c | 56 ++++++++++++++++++++++++++++++----------
>  drivers/base/power/opp/cpu.c  | 59 ++++++++++++++++++++++++++++++++-----------
>  include/linux/pm_opp.h        | 10 ++++++++
>  3 files changed, 96 insertions(+), 29 deletions(-)
> 
> v1->v2:
> 	- Instead of renaming OF versions, created non-OF versions of
> 	  dev_pm_opp_{cpumask_,}remove_table as suggested by Viresh
> 

You should have added v2->v3 here

> This version also updates all the errors in documentation and changes
> to use remove_all rather than remove_dyn.

But all that isn't going to be part of the history, so it should be fine :)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 1/2][UPDATE] PM / OPP: add non-OF versions of dev_pm_opp_{cpumask_,}remove_table

[Sudeep Holla]

On 29/04/16 10:28, Viresh Kumar wrote:
> This isn't V2, but V3
>
> On 29-04-16, 10:22, Sudeep Holla wrote:
>> Functions dev_pm_opp_of_{cpumask_,}remove_table removes/frees all the
>> static OPP entries associated with the device and/or all cpus(in case
>> of cpumask) that are created from DT.
>>
>> However the OPP entries are populated reading from the firmware or some
>> different method using dev_pm_opp_add are marked dynamic and can't be
>> removed using above functions.
>>
>> This patch adds non DT/OF versions of dev_pm_opp_{cpumask_,}remove_table
>> to support the above mentioned usecase.
>>
>> This is in preparation to make use of the same in scpi-cpufreq.c
>>
>> Cc: Viresh Kumar <vireshk@kernel.org>
>> Cc: Nishanth Menon <nm@ti.com>
>> CC: Stephen Boyd <sboyd@codeaurora.org>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: linux-pm@vger.kernel.org
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>   drivers/base/power/opp/core.c | 56 ++++++++++++++++++++++++++++++----------
>>   drivers/base/power/opp/cpu.c  | 59 ++++++++++++++++++++++++++++++++-----------
>>   include/linux/pm_opp.h        | 10 ++++++++
>>   3 files changed, 96 insertions(+), 29 deletions(-)
>>
>> v1->v2:
>> 	- Instead of renaming OF versions, created non-OF versions of
>> 	  dev_pm_opp_{cpumask_,}remove_table as suggested by Viresh
>>
>
> You should have added v2->v3 here
>

No, thought I will post v3 of both the patches with your ACK so that
it's easy for Rafael to pick up if he has no comments :)

>> This version also updates all the errors in documentation and changes
>> to use remove_all rather than remove_dyn.
>
> But all that isn't going to be part of the history, so it should be fine :)
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>

Thanks

-- 
Regards,
Sudeep

Re: [PATCH v2 1/2][UPDATE] PM / OPP: add non-OF versions of dev_pm_opp_{cpumask_,}remove_table

[Viresh Kumar]

On 29-04-16, 10:31, Sudeep Holla wrote:
> 
> 
> On 29/04/16 10:28, Viresh Kumar wrote:
> >This isn't V2, but V3
> >
> >On 29-04-16, 10:22, Sudeep Holla wrote:
> >>Functions dev_pm_opp_of_{cpumask_,}remove_table removes/frees all the
> >>static OPP entries associated with the device and/or all cpus(in case
> >>of cpumask) that are created from DT.
> >>
> >>However the OPP entries are populated reading from the firmware or some
> >>different method using dev_pm_opp_add are marked dynamic and can't be
> >>removed using above functions.
> >>
> >>This patch adds non DT/OF versions of dev_pm_opp_{cpumask_,}remove_table
> >>to support the above mentioned usecase.
> >>
> >>This is in preparation to make use of the same in scpi-cpufreq.c
> >>
> >>Cc: Viresh Kumar <vireshk@kernel.org>
> >>Cc: Nishanth Menon <nm@ti.com>
> >>CC: Stephen Boyd <sboyd@codeaurora.org>
> >>Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >>Cc: linux-pm@vger.kernel.org
> >>Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >>---
> >>  drivers/base/power/opp/core.c | 56 ++++++++++++++++++++++++++++++----------
> >>  drivers/base/power/opp/cpu.c  | 59 ++++++++++++++++++++++++++++++++-----------
> >>  include/linux/pm_opp.h        | 10 ++++++++
> >>  3 files changed, 96 insertions(+), 29 deletions(-)
> >>
> >>v1->v2:
> >>	- Instead of renaming OF versions, created non-OF versions of
> >>	  dev_pm_opp_{cpumask_,}remove_table as suggested by Viresh
> >>
> >
> >You should have added v2->v3 here
> >
> 
> No, thought I will post v3 of both the patches with your ACK so that
> it's easy for Rafael to pick up if he has no comments :)

Whatever you do, this was v3 unquestionably.

-- 
viresh