From 70b464918e5331e488058870fcc6821d54c4e541 Mon Sep 17 00:00:00 2001 From: Steve Twiss Date: Mon, 18 Mar 2019 16:17:57 +0000 Subject: [PATCH 1/5] regulator: core: fix error path for regulator_set_voltage_unlocked During several error paths in the function regulator_set_voltage_unlocked() the value of 'ret' can take on negative error values. However, in calls that go through the 'goto out' statement, this return value is lost and return 0 is used instead, indicating a 'pass'. There are several cases where this function should legitimately return a fail instead of a pass: one such case includes constraints check during voltage selection in the call to regulator_check_voltage(), which can have -EINVAL for the case when an unsupported voltage is incorrectly requested. In that case, -22 is expected as the return value, not 0. Fixes: 9243a195be7a ("regulator: core: Change voltage setting path") Cc: stable Signed-off-by: Steve Twiss Reviewed-by: Dmitry Osipenko Signed-off-by: Mark Brown --- drivers/regulator/core.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 68473d0cc57e..968dcd9d7a07 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -3322,15 +3322,12 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator, /* for not coupled regulators this will just set the voltage */ ret = regulator_balance_voltage(rdev, state); - if (ret < 0) - goto out2; + if (ret < 0) { + voltage->min_uV = old_min_uV; + voltage->max_uV = old_max_uV; + } out: - return 0; -out2: - voltage->min_uV = old_min_uV; - voltage->max_uV = old_max_uV; - return ret; } From 0b5e200cc7ee339694693a6d89816b66ccbd663a Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Sun, 24 Mar 2019 14:00:04 +0800 Subject: [PATCH 2/5] regulator: tps65217: Fix off-by-one for latest seletor of tps65217_uv1_ranges According to the datasheet the latest seletor 11 1111b = 3.3 V. Signed-off-by: Axel Lin Signed-off-by: Mark Brown --- drivers/regulator/tps65217-regulator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/regulator/tps65217-regulator.c b/drivers/regulator/tps65217-regulator.c index d84fab616abf..546fb5cf2023 100644 --- a/drivers/regulator/tps65217-regulator.c +++ b/drivers/regulator/tps65217-regulator.c @@ -61,7 +61,7 @@ static const struct regulator_linear_range tps65217_uv1_ranges[] = { REGULATOR_LINEAR_RANGE(1550000, 25, 30, 50000), REGULATOR_LINEAR_RANGE(1850000, 31, 52, 50000), REGULATOR_LINEAR_RANGE(3000000, 53, 55, 100000), - REGULATOR_LINEAR_RANGE(3300000, 56, 62, 0), + REGULATOR_LINEAR_RANGE(3300000, 56, 63, 0), }; static const struct regulator_linear_range tps65217_uv2_ranges[] = { From 063773011d33bb36588a90385aa9eb75d13c6d80 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Thu, 4 Apr 2019 16:32:18 +0100 Subject: [PATCH 3/5] regulator: core: Avoid potential deadlock on regulator_unregister Lockdep reports the following issue on my setup: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock((work_completion)(&(&rdev->disable_work)->work)); lock(regulator_list_mutex); lock((work_completion)(&(&rdev->disable_work)->work)); lock(regulator_list_mutex); The problem is that regulator_unregister takes the regulator_list_mutex and then calls flush_work on disable_work. But regulator_disable_work calls regulator_lock_dependent which will also take the regulator_list_mutex. Resulting in a deadlock if the flush_work call actually needs to flush the work. Fix this issue by moving the flush_work outside of the regulator_list_mutex. The list mutex is not used to guard the point at which the delayed work is queued, so its use adds no additional safety. Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Signed-off-by: Charles Keepax Reviewed-by: Dmitry Osipenko Signed-off-by: Mark Brown --- drivers/regulator/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 968dcd9d7a07..8573dd0871fd 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -5061,10 +5061,11 @@ void regulator_unregister(struct regulator_dev *rdev) regulator_put(rdev->supply); } + flush_work(&rdev->disable_work.work); + mutex_lock(®ulator_list_mutex); debugfs_remove_recursive(rdev->debugfs); - flush_work(&rdev->disable_work.work); WARN_ON(rdev->open_count); regulator_remove_coupling(rdev); unset_regulator_supplies(rdev); From 78927aa40bc82f32de07323ddc1c9de07ac68180 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Sat, 20 Apr 2019 13:34:30 +0200 Subject: [PATCH 4/5] regulator: core: Actually put the gpiod after use I went to great lengths to hand over the management of the GPIO descriptors to the regulator core, and some stray rebased oneliner in the old patch must have been assuming the devices were still doing devres management of it. We handed the management over to the regulator core, so of course the regulator core shall issue gpiod_put() when done. Sorry for the descriptor leak. Fixes: 541d052d7215 ("regulator: core: Only support passing enable GPIO descriptors") Signed-off-by: Linus Walleij Signed-off-by: Mark Brown --- drivers/regulator/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 8573dd0871fd..35a7d020afec 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -2256,6 +2256,7 @@ static void regulator_ena_gpio_free(struct regulator_dev *rdev) if (pin->gpiod == rdev->ena_pin->gpiod) { if (pin->request_count <= 1) { pin->request_count = 0; + gpiod_put(pin->gpiod); list_del(&pin->list); kfree(pin); rdev->ena_pin = NULL; From 734008e714d8c757613c43ff57c1bf7ef1bb5b5d Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Mon, 22 Apr 2019 13:35:31 +0800 Subject: [PATCH 5/5] regulator: max77620: Fix regulator info setting for max20024 Current code always set pmic->rinfo[id] = &max77620_regs_info[id]; It should set to either max77620_regs_info or max20024_regs_info depends on the chip_id. Signed-off-by: Axel Lin Reviewed-by: Dmitry Osipenko Tested-by: Dmitry Osipenko Signed-off-by: Mark Brown --- drivers/regulator/max77620-regulator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/regulator/max77620-regulator.c b/drivers/regulator/max77620-regulator.c index 1607ac673e44..0ad91a7f9cb9 100644 --- a/drivers/regulator/max77620-regulator.c +++ b/drivers/regulator/max77620-regulator.c @@ -803,7 +803,7 @@ static int max77620_regulator_probe(struct platform_device *pdev) continue; rdesc = &rinfo[id].desc; - pmic->rinfo[id] = &max77620_regs_info[id]; + pmic->rinfo[id] = &rinfo[id]; pmic->enable_power_mode[id] = MAX77620_POWER_MODE_NORMAL; pmic->reg_pdata[id].active_fps_src = -1; pmic->reg_pdata[id].active_fps_pd_slot = -1;