From d6d2a5289a530a3020703e6a3b19a14668601c27 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Sat, 17 Oct 2015 09:45:18 +0530 Subject: [PATCH 1/7] PM / OPP: Improve print messages with pr_fmt To identify OPP core's print messages easily, prefix them with KBUILD_MODNAME. Signed-off-by: Viresh Kumar Reviewed-by: Stephen Boyd Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp/core.c | 2 ++ drivers/base/power/opp/cpu.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index a731fa66e504..60ae6f029499 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -11,6 +11,8 @@ * published by the Free Software Foundation. */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c index 7654c5606307..c27a1cdffec9 100644 --- a/drivers/base/power/opp/cpu.c +++ b/drivers/base/power/opp/cpu.c @@ -10,6 +10,9 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include From b64b9c3f900a0522fb926f1436088e2e36807594 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 15 Oct 2015 21:42:44 +0530 Subject: [PATCH 2/7] PM / OPP: Rename routines specific to old bindings with _v1 Clearly distinguish routines based on what version of bindings they parse. We have already postfixed routines properly with _v2 for new bindings. Postfix the older ones now with _v1. Signed-off-by: Viresh Kumar Reviewed-by: Stephen Boyd Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp/core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 60ae6f029499..ca70e281614b 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -703,7 +703,7 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, } /** - * _opp_add_dynamic() - Allocate a dynamic OPP. + * _opp_add_v1() - Allocate a OPP based on v1 bindings. * @dev: device for which we do this operation * @freq: Frequency in Hz for this OPP * @u_volt: Voltage in uVolts for this OPP @@ -729,8 +729,8 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, * Duplicate OPPs (both freq and volt are same) and !opp->available * -ENOMEM Memory allocation failure */ -static int _opp_add_dynamic(struct device *dev, unsigned long freq, - long u_volt, bool dynamic) +static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, + bool dynamic) { struct device_opp *dev_opp; struct dev_pm_opp *new_opp; @@ -934,7 +934,7 @@ unlock: */ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt) { - return _opp_add_dynamic(dev, freq, u_volt, true); + return _opp_add_v1(dev, freq, u_volt, true); } EXPORT_SYMBOL_GPL(dev_pm_opp_add); @@ -1236,7 +1236,7 @@ static int _of_add_opp_table_v1(struct device *dev) unsigned long freq = be32_to_cpup(val++) * 1000; unsigned long volt = be32_to_cpup(val++); - if (_opp_add_dynamic(dev, freq, volt, false)) + if (_opp_add_v1(dev, freq, volt, false)) dev_warn(dev, "%s: Failed to add OPP %ld\n", __func__, freq); nr -= 2; From ad623c31485581d6b082ef92429db3b728739cd8 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 15 Oct 2015 21:42:45 +0530 Subject: [PATCH 3/7] PM / OPP: Parse all power-supply related bindings together Move all DT parsing for the power supplies to a single function, rather than keeping them at separate places. This will help manage things properly. Signed-off-by: Viresh Kumar Reviewed-by: Stephen Boyd Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp/core.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index ca70e281614b..ccf2c91aedff 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -772,9 +772,10 @@ unlock: } /* TODO: Support multiple regulators */ -static int opp_get_microvolt(struct dev_pm_opp *opp, struct device *dev) +static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) { u32 microvolt[3] = {0}; + u32 val; int count, ret; count = of_property_count_u32_elems(opp->np, "opp-microvolt"); @@ -800,6 +801,9 @@ static int opp_get_microvolt(struct dev_pm_opp *opp, struct device *dev) opp->u_volt_min = microvolt[1]; opp->u_volt_max = microvolt[2]; + if (!of_property_read_u32(opp->np, "opp-microamp", &val)) + opp->u_amp = val; + return 0; } @@ -864,13 +868,10 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) if (!of_property_read_u32(np, "clock-latency-ns", &val)) new_opp->clock_latency_ns = val; - ret = opp_get_microvolt(new_opp, dev); + ret = opp_parse_supplies(new_opp, dev); if (ret) goto free_opp; - if (!of_property_read_u32(new_opp->np, "opp-microamp", &val)) - new_opp->u_amp = val; - ret = _opp_add(dev, new_opp, dev_opp); if (ret) goto free_opp; From 1794ec1f9585501e4ed4390f5a5d396fd28c63ce Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 5 Nov 2015 14:21:18 +0530 Subject: [PATCH 4/7] PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus() We are returning 0 even in case of errors, fix it. Fixes: 8d4d4e98acd6 ("PM / OPP: Add helpers for initializing CPU OPPs") Reported-by: Dan Carpenter Reviewed-by: Stephen Boyd Signed-off-by: Viresh Kumar Cc: 4.3 # 4.3 Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c index c27a1cdffec9..2139c9d4c447 100644 --- a/drivers/base/power/opp/cpu.c +++ b/drivers/base/power/opp/cpu.c @@ -156,7 +156,7 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask) out_rcu_read_unlock: rcu_read_unlock(); - return 0; + return ret; } EXPORT_SYMBOL_GPL(dev_pm_opp_set_sharing_cpus); From 87b4115db0239865bc812f61704bb1f43e2439b6 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 5 Nov 2015 14:21:19 +0530 Subject: [PATCH 5/7] PM / OPP: Protect updates to list_dev with mutex dev_opp_list_lock is used everywhere to protect device and OPP lists, but dev_pm_opp_set_sharing_cpus() is missed somehow. And instead we used rcu-lock, which wouldn't help here as we are adding a new list_dev. This also fixes a problem where we have called kzalloc(..., GFP_KERNEL) from within rcu-lock, which isn't allowed as kzalloc can sleep when called with GFP_KERNEL. With CONFIG_DEBUG_ATOMIC_SLEEP set, we get following lockdep-splat: include/linux/rcupdate.h:578 Illegal context switch in RCU read-side critical section! other info that might help us debug this: rcu_scheduler_active = 1, debug_locks = 0 5 locks held by swapper/0/1: #0: (&dev->mutex){......}, at: [] __driver_attach+0x48/0x98 #1: (&dev->mutex){......}, at: [] __driver_attach+0x58/0x98 #2: (cpu_hotplug.lock){++++++}, at: [] get_online_cpus+0x40/0xb0 #3: (subsys mutex#5){+.+.+.}, at: [] subsys_interface_register+0x44/0xdc #4: (rcu_read_lock){......}, at: [] dev_pm_opp_set_sharing_cpus+0x0/0x1e4 stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 4.3.0-rc7-00047-g81f5932958a8 #59 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x94/0xbc) [] (dump_stack) from [] (___might_sleep+0x24c/0x298) [] (___might_sleep) from [] (kmem_cache_alloc+0xe8/0x164) [] (kmem_cache_alloc) from [] (_add_list_dev+0x30/0x58) [] (_add_list_dev) from [] (dev_pm_opp_set_sharing_cpus+0xd0/0x1e4) [] (dev_pm_opp_set_sharing_cpus) from [] (cpufreq_init+0x4cc/0x62c) [] (cpufreq_init) from [] (cpufreq_online+0xbc/0x73c) [] (cpufreq_online) from [] (subsys_interface_register+0x98/0xdc) [] (subsys_interface_register) from [] (cpufreq_register_driver+0x110/0x17c) [] (cpufreq_register_driver) from [] (dt_cpufreq_probe+0x60/0x8c) [] (dt_cpufreq_probe) from [] (platform_drv_probe+0x44/0xa4) [] (platform_drv_probe) from [] (driver_probe_device+0x208/0x2f4) [] (driver_probe_device) from [] (__driver_attach+0x94/0x98) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0x9c) Reported-by: Michael Turquette Reviewed-by: Stephen Boyd Signed-off-by: Viresh Kumar Cc: 4.3 # 4.3 Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp/core.c | 2 +- drivers/base/power/opp/cpu.c | 8 ++++---- drivers/base/power/opp/opp.h | 3 +++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index ccf2c91aedff..252706d6f60b 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -29,7 +29,7 @@ */ static LIST_HEAD(dev_opp_list); /* Lock to allow exclusive modification to the device and opp lists */ -static DEFINE_MUTEX(dev_opp_list_lock); +DEFINE_MUTEX(dev_opp_list_lock); #define opp_rcu_lockdep_assert() \ do { \ diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c index 2139c9d4c447..7b445e88a0d5 100644 --- a/drivers/base/power/opp/cpu.c +++ b/drivers/base/power/opp/cpu.c @@ -127,12 +127,12 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask) struct device *dev; int cpu, ret = 0; - rcu_read_lock(); + mutex_lock(&dev_opp_list_lock); dev_opp = _find_device_opp(cpu_dev); if (IS_ERR(dev_opp)) { ret = -EINVAL; - goto out_rcu_read_unlock; + goto unlock; } for_each_cpu(cpu, cpumask) { @@ -153,8 +153,8 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask) continue; } } -out_rcu_read_unlock: - rcu_read_unlock(); +unlock: + mutex_unlock(&dev_opp_list_lock); return ret; } diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index dcb38f78dae4..7366b2aa8997 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -21,6 +21,9 @@ #include #include +/* Lock to allow exclusive modification to the device and opp lists */ +extern struct mutex dev_opp_list_lock; + /* * Internal data structure organization with the OPP layer library is as * follows: From 4a3a1353a84796f93d389694e3b87ede533953fe Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 5 Nov 2015 14:21:20 +0530 Subject: [PATCH 6/7] PM / OPP: Hold dev_opp_list_lock for writers Writers need to update OPP device and their list with dev_opp_list_lock mutex held, which was missed at few places. Fix it. Signed-off-by: Viresh Kumar Cc: 4.3 # 4.3 Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp/core.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 252706d6f60b..d5f215679820 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -1168,13 +1168,17 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) struct device_opp *dev_opp; int ret = 0, count = 0; + mutex_lock(&dev_opp_list_lock); + dev_opp = _managed_opp(opp_np); if (dev_opp) { /* OPPs are already managed */ if (!_add_list_dev(dev, dev_opp)) ret = -ENOMEM; + mutex_unlock(&dev_opp_list_lock); return ret; } + mutex_unlock(&dev_opp_list_lock); /* We have opp-list node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_np, np) { @@ -1192,15 +1196,20 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) if (WARN_ON(!count)) return -ENOENT; + mutex_lock(&dev_opp_list_lock); + dev_opp = _find_device_opp(dev); if (WARN_ON(IS_ERR(dev_opp))) { ret = PTR_ERR(dev_opp); + mutex_unlock(&dev_opp_list_lock); goto free_table; } dev_opp->np = opp_np; dev_opp->shared_opp = of_property_read_bool(opp_np, "opp-shared"); + mutex_unlock(&dev_opp_list_lock); + return 0; free_table: From 0597e818501f595090a49a1779ab6ec377051b11 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 5 Nov 2015 14:21:21 +0530 Subject: [PATCH 7/7] PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp() _find_device_opp() should be called with rcu-read lock or dev_opp_list_lock held. Add the opp_rcu_lockdep_assert() check to make sure caller have taken appropriate locks. Fix comment over the routine as well. Suggested-by: Stephen Boyd Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp/core.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index d5f215679820..c987d2e1a83b 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -81,14 +81,18 @@ static struct device_opp *_managed_opp(const struct device_node *np) * Return: pointer to 'struct device_opp' if found, otherwise -ENODEV or * -EINVAL based on type of error. * - * Locking: This function must be called under rcu_read_lock(). device_opp - * is a RCU protected pointer. This means that device_opp is valid as long - * as we are under RCU lock. + * Locking: For readers, this function must be called under rcu_read_lock(). + * device_opp is a RCU protected pointer, which means that device_opp is valid + * as long as we are under RCU lock. + * + * For Writers, this function must be called with dev_opp_list_lock held. */ struct device_opp *_find_device_opp(struct device *dev) { struct device_opp *dev_opp; + opp_rcu_lockdep_assert(); + if (IS_ERR_OR_NULL(dev)) { pr_err("%s: Invalid parameters\n", __func__); return ERR_PTR(-EINVAL);