linux/drivers/regulator/vctrl-regulator.c

555 lines
13 KiB
C
Raw Normal View History

// SPDX-License-Identifier: GPL-2.0-only
/*
* Driver for voltage controller regulators
*
* Copyright (C) 2017 Google, Inc.
*/
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/regulator/coupler.h>
#include <linux/regulator/driver.h>
#include <linux/regulator/of_regulator.h>
#include <linux/sort.h>
#include "internal.h"
struct vctrl_voltage_range {
int min_uV;
int max_uV;
};
struct vctrl_voltage_ranges {
struct vctrl_voltage_range ctrl;
struct vctrl_voltage_range out;
};
struct vctrl_voltage_table {
int ctrl;
int out;
int ovp_min_sel;
};
struct vctrl_data {
struct regulator_dev *rdev;
struct regulator_desc desc;
bool enabled;
unsigned int min_slew_down_rate;
unsigned int ovp_threshold;
struct vctrl_voltage_ranges vrange;
struct vctrl_voltage_table *vtable;
unsigned int sel;
};
static int vctrl_calc_ctrl_voltage(struct vctrl_data *vctrl, int out_uV)
{
struct vctrl_voltage_range *ctrl = &vctrl->vrange.ctrl;
struct vctrl_voltage_range *out = &vctrl->vrange.out;
return ctrl->min_uV +
DIV_ROUND_CLOSEST_ULL((s64)(out_uV - out->min_uV) *
(ctrl->max_uV - ctrl->min_uV),
out->max_uV - out->min_uV);
}
static int vctrl_calc_output_voltage(struct vctrl_data *vctrl, int ctrl_uV)
{
struct vctrl_voltage_range *ctrl = &vctrl->vrange.ctrl;
struct vctrl_voltage_range *out = &vctrl->vrange.out;
if (ctrl_uV < 0) {
pr_err("vctrl: failed to get control voltage\n");
return ctrl_uV;
}
if (ctrl_uV < ctrl->min_uV)
return out->min_uV;
if (ctrl_uV > ctrl->max_uV)
return out->max_uV;
return out->min_uV +
DIV_ROUND_CLOSEST_ULL((s64)(ctrl_uV - ctrl->min_uV) *
(out->max_uV - out->min_uV),
ctrl->max_uV - ctrl->min_uV);
}
static int vctrl_get_voltage(struct regulator_dev *rdev)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
int ctrl_uV;
if (!rdev->supply)
return -EPROBE_DEFER;
ctrl_uV = regulator_get_voltage_rdev(rdev->supply->rdev);
return vctrl_calc_output_voltage(vctrl, ctrl_uV);
}
static int vctrl_set_voltage(struct regulator_dev *rdev,
int req_min_uV, int req_max_uV,
unsigned int *selector)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
int orig_ctrl_uV;
int uV;
int ret;
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
if (!rdev->supply)
return -EPROBE_DEFER;
orig_ctrl_uV = regulator_get_voltage_rdev(rdev->supply->rdev);
uV = vctrl_calc_output_voltage(vctrl, orig_ctrl_uV);
if (req_min_uV >= uV || !vctrl->ovp_threshold)
/* voltage rising or no OVP */
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
return regulator_set_voltage_rdev(rdev->supply->rdev,
vctrl_calc_ctrl_voltage(vctrl, req_min_uV),
vctrl_calc_ctrl_voltage(vctrl, req_max_uV),
PM_SUSPEND_ON);
while (uV > req_min_uV) {
int max_drop_uV = (uV * vctrl->ovp_threshold) / 100;
int next_uV;
int next_ctrl_uV;
int delay;
/* Make sure no infinite loop even in crazy cases */
if (max_drop_uV == 0)
max_drop_uV = 1;
next_uV = max_t(int, req_min_uV, uV - max_drop_uV);
next_ctrl_uV = vctrl_calc_ctrl_voltage(vctrl, next_uV);
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
ret = regulator_set_voltage_rdev(rdev->supply->rdev,
next_ctrl_uV,
next_ctrl_uV,
PM_SUSPEND_ON);
if (ret)
goto err;
delay = DIV_ROUND_UP(uV - next_uV, vctrl->min_slew_down_rate);
usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
uV = next_uV;
}
return 0;
err:
/* Try to go back to original voltage */
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
regulator_set_voltage_rdev(rdev->supply->rdev, orig_ctrl_uV, orig_ctrl_uV,
PM_SUSPEND_ON);
return ret;
}
static int vctrl_get_voltage_sel(struct regulator_dev *rdev)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
return vctrl->sel;
}
static int vctrl_set_voltage_sel(struct regulator_dev *rdev,
unsigned int selector)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
unsigned int orig_sel = vctrl->sel;
int ret;
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
if (!rdev->supply)
return -EPROBE_DEFER;
if (selector >= rdev->desc->n_voltages)
return -EINVAL;
if (selector >= vctrl->sel || !vctrl->ovp_threshold) {
/* voltage rising or no OVP */
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
ret = regulator_set_voltage_rdev(rdev->supply->rdev,
vctrl->vtable[selector].ctrl,
vctrl->vtable[selector].ctrl,
PM_SUSPEND_ON);
if (!ret)
vctrl->sel = selector;
return ret;
}
while (vctrl->sel != selector) {
unsigned int next_sel;
int delay;
next_sel = max_t(unsigned int, selector, vctrl->vtable[vctrl->sel].ovp_min_sel);
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
ret = regulator_set_voltage_rdev(rdev->supply->rdev,
vctrl->vtable[next_sel].ctrl,
vctrl->vtable[next_sel].ctrl,
PM_SUSPEND_ON);
if (ret) {
dev_err(&rdev->dev,
"failed to set control voltage to %duV\n",
vctrl->vtable[next_sel].ctrl);
goto err;
}
vctrl->sel = next_sel;
delay = DIV_ROUND_UP(vctrl->vtable[vctrl->sel].out -
vctrl->vtable[next_sel].out,
vctrl->min_slew_down_rate);
usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
}
return 0;
err:
if (vctrl->sel != orig_sel) {
/* Try to go back to original voltage */
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
if (!regulator_set_voltage_rdev(rdev->supply->rdev,
vctrl->vtable[orig_sel].ctrl,
vctrl->vtable[orig_sel].ctrl,
PM_SUSPEND_ON))
vctrl->sel = orig_sel;
else
dev_warn(&rdev->dev,
"failed to restore original voltage\n");
}
return ret;
}
static int vctrl_list_voltage(struct regulator_dev *rdev,
unsigned int selector)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
if (selector >= rdev->desc->n_voltages)
return -EINVAL;
return vctrl->vtable[selector].out;
}
static int vctrl_parse_dt(struct platform_device *pdev,
struct vctrl_data *vctrl)
{
int ret;
struct device_node *np = pdev->dev.of_node;
u32 pval;
u32 vrange_ctrl[2];
ret = of_property_read_u32(np, "ovp-threshold-percent", &pval);
if (!ret) {
vctrl->ovp_threshold = pval;
if (vctrl->ovp_threshold > 100) {
dev_err(&pdev->dev,
"ovp-threshold-percent (%u) > 100\n",
vctrl->ovp_threshold);
return -EINVAL;
}
}
ret = of_property_read_u32(np, "min-slew-down-rate", &pval);
if (!ret) {
vctrl->min_slew_down_rate = pval;
/* We use the value as int and as divider; sanity check */
if (vctrl->min_slew_down_rate == 0) {
dev_err(&pdev->dev,
"min-slew-down-rate must not be 0\n");
return -EINVAL;
} else if (vctrl->min_slew_down_rate > INT_MAX) {
dev_err(&pdev->dev, "min-slew-down-rate (%u) too big\n",
vctrl->min_slew_down_rate);
return -EINVAL;
}
}
if (vctrl->ovp_threshold && !vctrl->min_slew_down_rate) {
dev_err(&pdev->dev,
"ovp-threshold-percent requires min-slew-down-rate\n");
return -EINVAL;
}
ret = of_property_read_u32(np, "regulator-min-microvolt", &pval);
if (ret) {
dev_err(&pdev->dev,
"failed to read regulator-min-microvolt: %d\n", ret);
return ret;
}
vctrl->vrange.out.min_uV = pval;
ret = of_property_read_u32(np, "regulator-max-microvolt", &pval);
if (ret) {
dev_err(&pdev->dev,
"failed to read regulator-max-microvolt: %d\n", ret);
return ret;
}
vctrl->vrange.out.max_uV = pval;
ret = of_property_read_u32_array(np, "ctrl-voltage-range", vrange_ctrl,
2);
if (ret) {
dev_err(&pdev->dev, "failed to read ctrl-voltage-range: %d\n",
ret);
return ret;
}
if (vrange_ctrl[0] >= vrange_ctrl[1]) {
dev_err(&pdev->dev, "ctrl-voltage-range is invalid: %d-%d\n",
vrange_ctrl[0], vrange_ctrl[1]);
return -EINVAL;
}
vctrl->vrange.ctrl.min_uV = vrange_ctrl[0];
vctrl->vrange.ctrl.max_uV = vrange_ctrl[1];
return 0;
}
static int vctrl_cmp_ctrl_uV(const void *a, const void *b)
{
const struct vctrl_voltage_table *at = a;
const struct vctrl_voltage_table *bt = b;
return at->ctrl - bt->ctrl;
}
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
static int vctrl_init_vtable(struct platform_device *pdev,
struct regulator *ctrl_reg)
{
struct vctrl_data *vctrl = platform_get_drvdata(pdev);
struct regulator_desc *rdesc = &vctrl->desc;
struct vctrl_voltage_range *vrange_ctrl = &vctrl->vrange.ctrl;
int n_voltages;
int ctrl_uV;
int i, idx_vt;
n_voltages = regulator_count_voltages(ctrl_reg);
rdesc->n_voltages = n_voltages;
/* determine number of steps within the range of the vctrl regulator */
for (i = 0; i < n_voltages; i++) {
ctrl_uV = regulator_list_voltage(ctrl_reg, i);
if (ctrl_uV < vrange_ctrl->min_uV ||
ctrl_uV > vrange_ctrl->max_uV)
rdesc->n_voltages--;
}
if (rdesc->n_voltages == 0) {
dev_err(&pdev->dev, "invalid configuration\n");
return -EINVAL;
}
vctrl->vtable = devm_kcalloc(&pdev->dev, rdesc->n_voltages,
sizeof(struct vctrl_voltage_table),
GFP_KERNEL);
if (!vctrl->vtable)
return -ENOMEM;
/* create mapping control <=> output voltage */
for (i = 0, idx_vt = 0; i < n_voltages; i++) {
ctrl_uV = regulator_list_voltage(ctrl_reg, i);
if (ctrl_uV < vrange_ctrl->min_uV ||
ctrl_uV > vrange_ctrl->max_uV)
continue;
vctrl->vtable[idx_vt].ctrl = ctrl_uV;
vctrl->vtable[idx_vt].out =
vctrl_calc_output_voltage(vctrl, ctrl_uV);
idx_vt++;
}
/* we rely on the table to be ordered by ascending voltage */
sort(vctrl->vtable, rdesc->n_voltages,
sizeof(struct vctrl_voltage_table), vctrl_cmp_ctrl_uV,
NULL);
/* pre-calculate OVP-safe downward transitions */
for (i = rdesc->n_voltages - 1; i > 0; i--) {
int j;
int ovp_min_uV = (vctrl->vtable[i].out *
(100 - vctrl->ovp_threshold)) / 100;
for (j = 0; j < i; j++) {
if (vctrl->vtable[j].out >= ovp_min_uV) {
vctrl->vtable[i].ovp_min_sel = j;
break;
}
}
if (j == i) {
dev_warn(&pdev->dev, "switching down from %duV may cause OVP shutdown\n",
vctrl->vtable[i].out);
/* use next lowest voltage */
vctrl->vtable[i].ovp_min_sel = i - 1;
}
}
return 0;
}
static int vctrl_enable(struct regulator_dev *rdev)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
vctrl->enabled = true;
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
return 0;
}
static int vctrl_disable(struct regulator_dev *rdev)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
vctrl->enabled = false;
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
return 0;
}
static int vctrl_is_enabled(struct regulator_dev *rdev)
{
struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
return vctrl->enabled;
}
static const struct regulator_ops vctrl_ops_cont = {
.enable = vctrl_enable,
.disable = vctrl_disable,
.is_enabled = vctrl_is_enabled,
.get_voltage = vctrl_get_voltage,
.set_voltage = vctrl_set_voltage,
};
static const struct regulator_ops vctrl_ops_non_cont = {
.enable = vctrl_enable,
.disable = vctrl_disable,
.is_enabled = vctrl_is_enabled,
.set_voltage_sel = vctrl_set_voltage_sel,
.get_voltage_sel = vctrl_get_voltage_sel,
.list_voltage = vctrl_list_voltage,
.map_voltage = regulator_map_voltage_iterate,
};
static int vctrl_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct vctrl_data *vctrl;
const struct regulator_init_data *init_data;
struct regulator_desc *rdesc;
struct regulator_config cfg = { };
struct vctrl_voltage_range *vrange_ctrl;
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
struct regulator *ctrl_reg;
int ctrl_uV;
int ret;
vctrl = devm_kzalloc(&pdev->dev, sizeof(struct vctrl_data),
GFP_KERNEL);
if (!vctrl)
return -ENOMEM;
platform_set_drvdata(pdev, vctrl);
ret = vctrl_parse_dt(pdev, vctrl);
if (ret)
return ret;
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
ctrl_reg = devm_regulator_get(&pdev->dev, "ctrl");
if (IS_ERR(ctrl_reg))
return PTR_ERR(ctrl_reg);
vrange_ctrl = &vctrl->vrange.ctrl;
rdesc = &vctrl->desc;
rdesc->name = "vctrl";
rdesc->type = REGULATOR_VOLTAGE;
rdesc->owner = THIS_MODULE;
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
rdesc->supply_name = "ctrl";
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
if ((regulator_get_linear_step(ctrl_reg) == 1) ||
(regulator_count_voltages(ctrl_reg) == -EINVAL)) {
rdesc->continuous_voltage_range = true;
rdesc->ops = &vctrl_ops_cont;
} else {
rdesc->ops = &vctrl_ops_non_cont;
}
init_data = of_get_regulator_init_data(&pdev->dev, np, rdesc);
if (!init_data)
return -ENOMEM;
cfg.of_node = np;
cfg.dev = &pdev->dev;
cfg.driver_data = vctrl;
cfg.init_data = init_data;
if (!rdesc->continuous_voltage_range) {
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
ret = vctrl_init_vtable(pdev, ctrl_reg);
if (ret)
return ret;
/* Use locked consumer API when not in regulator framework */
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
ctrl_uV = regulator_get_voltage(ctrl_reg);
if (ctrl_uV < 0) {
dev_err(&pdev->dev, "failed to get control voltage\n");
return ctrl_uV;
}
/* determine current voltage selector from control voltage */
if (ctrl_uV < vrange_ctrl->min_uV) {
vctrl->sel = 0;
} else if (ctrl_uV > vrange_ctrl->max_uV) {
vctrl->sel = rdesc->n_voltages - 1;
} else {
int i;
for (i = 0; i < rdesc->n_voltages; i++) {
if (ctrl_uV == vctrl->vtable[i].ctrl) {
vctrl->sel = i;
break;
}
}
}
}
regulator: vctrl: Avoid lockdep warning in enable/disable ops vctrl_enable() and vctrl_disable() call regulator_enable() and regulator_disable(), respectively. However, vctrl_* are regulator ops and should not be calling the locked regulator APIs. Doing so results in a lockdep warning. Instead of exporting more internal regulator ops, model the ctrl supply as an actual supply to vctrl-regulator. At probe time this driver still needs to use the consumer API to fetch its constraints, but otherwise lets the regulator core handle the upstream supply for it. The enable/disable/is_enabled ops are not removed, but now only track state internally. This preserves the original behavior with the ops being available, but one could argue that the original behavior was already incorrect: the internal state would not match the upstream supply if that supply had another consumer that enabled the supply, while vctrl-regulator was not enabled. The lockdep warning is as follows: WARNING: possible circular locking dependency detected 5.14.0-rc6 #2 Not tainted ------------------------------------------------------ swapper/0/1 is trying to acquire lock: ffffffc011306d00 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) but task is already holding lock: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) ww_mutex_lock (kernel/locking/mutex.c:1199) regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) regulator_lock_dependent (drivers/regulator/core.c:343) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) platform_probe (drivers/base/platform.c:1427) [...] -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_lock_dependent (include/linux/ww_mutex.h:129 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) set_machine_constraints (drivers/regulator/core.c:1536) regulator_register (drivers/regulator/core.c:5486) devm_regulator_register (drivers/regulator/devres.c:196) reg_fixed_voltage_probe (drivers/regulator/fixed.c:289) [...] -> #0 (regulator_list_mutex){+.+.}-{3:3}: __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] other info that might help us debug this: Chain exists of: regulator_list_mutex --> regulator_ww_class_acquire --> regulator_ww_class_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_acquire); lock(regulator_ww_class_mutex); lock(regulator_list_mutex); *** DEADLOCK *** 6 locks held by swapper/0/1: #0: ffffff8002d32188 (&dev->mutex){....}-{3:3}, at: __device_driver_lock (drivers/base/dd.c:1030) #1: ffffffc0111a0520 (cpu_hotplug_lock){++++}-{0:0}, at: cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2792 (discriminator 2)) #2: ffffff8002a8d918 (subsys mutex#9){+.+.}-{3:3}, at: subsys_interface_register (drivers/base/bus.c:1033) #3: ffffff800341bb90 (&policy->rwsem){+.+.}-{3:3}, at: cpufreq_online (include/linux/bitmap.h:285 include/linux/cpumask.h:405 drivers/cpufreq/cpufreq.c:1399) #4: ffffffc011f0b7b8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable (drivers/regulator/core.c:2808) #5: ffffff8004a77160 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_lock_recursive (drivers/regulator/core.c:156 drivers/regulator/core.c:263) stack backtrace: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc6 #2 7c8f8996d021ed0f65271e6aeebf7999de74a9fa Hardware name: Google Scarlet (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:161) show_stack (arch/arm64/kernel/stacktrace.c:218) dump_stack_lvl (lib/dump_stack.c:106 (discriminator 2)) dump_stack (lib/dump_stack.c:113) print_circular_bug (kernel/locking/lockdep.c:?) check_noncircular (kernel/locking/lockdep.c:?) __lock_acquire (kernel/locking/lockdep.c:3052 (discriminator 4) kernel/locking/lockdep.c:3174 (discriminator 4) kernel/locking/lockdep.c:3789 (discriminator 4) kernel/locking/lockdep.c:5015 (discriminator 4)) lock_acquire (arch/arm64/include/asm/percpu.h:39 kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5627) __mutex_lock_common (include/asm-generic/atomic-instrumented.h:606 include/asm-generic/atomic-long.h:29 kernel/locking/mutex.c:103 kernel/locking/mutex.c:144 kernel/locking/mutex.c:963) mutex_lock_nested (kernel/locking/mutex.c:1125) regulator_lock_dependent (arch/arm64/include/asm/current.h:19 include/linux/ww_mutex.h:111 drivers/regulator/core.c:329) regulator_enable (drivers/regulator/core.c:2808) vctrl_enable (drivers/regulator/vctrl-regulator.c:400) _regulator_do_enable (drivers/regulator/core.c:2617) _regulator_enable (drivers/regulator/core.c:2764) regulator_enable (drivers/regulator/core.c:308 drivers/regulator/core.c:2809) _set_opp (drivers/opp/core.c:819 drivers/opp/core.c:1072) dev_pm_opp_set_rate (drivers/opp/core.c:1164) set_target (drivers/cpufreq/cpufreq-dt.c:62) __cpufreq_driver_target (drivers/cpufreq/cpufreq.c:2216 drivers/cpufreq/cpufreq.c:2271) cpufreq_online (drivers/cpufreq/cpufreq.c:1488 (discriminator 2)) cpufreq_add_dev (drivers/cpufreq/cpufreq.c:1563) subsys_interface_register (drivers/base/bus.c:?) cpufreq_register_driver (drivers/cpufreq/cpufreq.c:2819) dt_cpufreq_probe (drivers/cpufreq/cpufreq-dt.c:344) [...] Reported-by: Brian Norris <briannorris@chromium.org> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Fixes: e9153311491d ("regulator: vctrl-regulator: Avoid deadlock getting and setting the voltage") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20210825033704.3307263-3-wenst@chromium.org Signed-off-by: Mark Brown <broonie@kernel.org>
2021-08-25 03:37:04 +00:00
/* Drop ctrl-supply here in favor of regulator core managed supply */
devm_regulator_put(ctrl_reg);
vctrl->rdev = devm_regulator_register(&pdev->dev, rdesc, &cfg);
if (IS_ERR(vctrl->rdev)) {
ret = PTR_ERR(vctrl->rdev);
dev_err(&pdev->dev, "failed to register regulator: %d\n", ret);
return ret;
}
return 0;
}
static const struct of_device_id vctrl_of_match[] = {
{ .compatible = "vctrl-regulator", },
{},
};
MODULE_DEVICE_TABLE(of, vctrl_of_match);
static struct platform_driver vctrl_driver = {
.probe = vctrl_probe,
.driver = {
.name = "vctrl-regulator",
.of_match_table = of_match_ptr(vctrl_of_match),
},
};
module_platform_driver(vctrl_driver);
MODULE_DESCRIPTION("Voltage Controlled Regulator Driver");
MODULE_AUTHOR("Matthias Kaehlcke <mka@chromium.org>");
MODULE_LICENSE("GPL v2");