From 0955f5be4337c0ada82e49389249eb99199f8db2 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 4 Aug 2020 00:08:37 -0700 Subject: regulator: Avoid grabbing regulator lock during suspend/resume I see it takes about 5us per regulator to grab the lock, check that this regulator isn't going to do anything for suspend, and then release the lock. When that is combined with PMICs that have dozens of regulators we get into a state where we spend a few miliseconds doing a bunch of locking operations synchronously to figure out that there's nothing to do. Let's reorganize the code here a bit so that we don't grab the lock until we're actually going to do something so that suspend is a little faster. Signed-off-by: Stephen Boyd Reviewed-by: Douglas Anderson Cc: Matthias Kaehlcke Cc: Douglas Anderson Link: https://lore.kernel.org/r/20200804070837.1084024-1-swboyd@chromium.org Signed-off-by: Mark Brown --- drivers/regulator/core.c | 75 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 24 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 75ff7c563c5d..76e9ed884afa 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -565,6 +565,30 @@ regulator_get_suspend_state(struct regulator_dev *rdev, suspend_state_t state) } } +static const struct regulator_state * +regulator_get_suspend_state_check(struct regulator_dev *rdev, suspend_state_t state) +{ + const struct regulator_state *rstate; + + rstate = regulator_get_suspend_state(rdev, state); + if (rstate == NULL) + return NULL; + + /* If we have no suspend mode configuration don't set anything; + * only warn if the driver implements set_suspend_voltage or + * set_suspend_mode callback. + */ + if (rstate->enabled != ENABLE_IN_SUSPEND && + rstate->enabled != DISABLE_IN_SUSPEND) { + if (rdev->desc->ops->set_suspend_voltage || + rdev->desc->ops->set_suspend_mode) + rdev_warn(rdev, "No configuration\n"); + return NULL; + } + + return rstate; +} + static ssize_t regulator_uV_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -982,27 +1006,10 @@ static int drms_uA_update(struct regulator_dev *rdev) return err; } -static int suspend_set_state(struct regulator_dev *rdev, - suspend_state_t state) +static int __suspend_set_state(struct regulator_dev *rdev, + const struct regulator_state *rstate) { int ret = 0; - struct regulator_state *rstate; - - rstate = regulator_get_suspend_state(rdev, state); - if (rstate == NULL) - return 0; - - /* If we have no suspend mode configuration don't set anything; - * only warn if the driver implements set_suspend_voltage or - * set_suspend_mode callback. - */ - if (rstate->enabled != ENABLE_IN_SUSPEND && - rstate->enabled != DISABLE_IN_SUSPEND) { - if (rdev->desc->ops->set_suspend_voltage || - rdev->desc->ops->set_suspend_mode) - rdev_warn(rdev, "No configuration\n"); - return 0; - } if (rstate->enabled == ENABLE_IN_SUSPEND && rdev->desc->ops->set_suspend_enable) @@ -1037,6 +1044,18 @@ static int suspend_set_state(struct regulator_dev *rdev, return ret; } +static int suspend_set_initial_state(struct regulator_dev *rdev) +{ + const struct regulator_state *rstate; + + rstate = regulator_get_suspend_state_check(rdev, + rdev->constraints->initial_state); + if (!rstate) + return 0; + + return __suspend_set_state(rdev, rstate); +} + static void print_constraints(struct regulator_dev *rdev) { struct regulation_constraints *constraints = rdev->constraints; @@ -1319,7 +1338,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, /* do we need to setup our suspend state */ if (rdev->constraints->initial_state) { - ret = suspend_set_state(rdev, rdev->constraints->initial_state); + ret = suspend_set_initial_state(rdev); if (ret < 0) { rdev_err(rdev, "failed to set suspend state\n"); return ret; @@ -5362,9 +5381,14 @@ static int regulator_suspend(struct device *dev) struct regulator_dev *rdev = dev_to_rdev(dev); suspend_state_t state = pm_suspend_target_state; int ret; + const struct regulator_state *rstate; + + rstate = regulator_get_suspend_state_check(rdev, state); + if (!rstate) + return 0; regulator_lock(rdev); - ret = suspend_set_state(rdev, state); + ret = __suspend_set_state(rdev, rstate); regulator_unlock(rdev); return ret; @@ -5381,11 +5405,14 @@ static int regulator_resume(struct device *dev) if (rstate == NULL) return 0; + /* Avoid grabbing the lock if we don't need to */ + if (!rdev->desc->ops->resume) + return 0; + regulator_lock(rdev); - if (rdev->desc->ops->resume && - (rstate->enabled == ENABLE_IN_SUSPEND || - rstate->enabled == DISABLE_IN_SUSPEND)) + if (rstate->enabled == ENABLE_IN_SUSPEND || + rstate->enabled == DISABLE_IN_SUSPEND) ret = rdev->desc->ops->resume(rdev); regulator_unlock(rdev); -- cgit From 7d8196641ee1eea6223e68ff92362652b1c8346b Mon Sep 17 00:00:00 2001 From: Michał Mirosław Date: Sun, 9 Aug 2020 21:21:16 +0200 Subject: regulator: Remove pointer table overallocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code allocates sizeof(regulator_dev) for a pointer. Make it less generous. Let kcalloc() calculate the size, while at it. Signed-off-by: Michał Mirosław Reviewed-by: Dmitry Osipenko Link: https://lore.kernel.org/r/407fbd06a02caf038a9ba3baa51c7d6d47cd6517.1597000795.git.mirq-linux@rere.qmqm.pl Signed-off-by: Mark Brown --- drivers/regulator/core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 76e9ed884afa..98777d720063 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -5030,20 +5030,20 @@ static void regulator_remove_coupling(struct regulator_dev *rdev) static int regulator_init_coupling(struct regulator_dev *rdev) { + struct regulator_dev **coupled; int err, n_phandles; - size_t alloc_size; if (!IS_ENABLED(CONFIG_OF)) n_phandles = 0; else n_phandles = of_get_n_coupled(rdev); - alloc_size = sizeof(*rdev) * (n_phandles + 1); - - rdev->coupling_desc.coupled_rdevs = kzalloc(alloc_size, GFP_KERNEL); - if (!rdev->coupling_desc.coupled_rdevs) + coupled = kcalloc(n_phandles + 1, sizeof(*coupled), GFP_KERNEL); + if (!coupled) return -ENOMEM; + rdev->coupling_desc.coupled_rdevs = coupled; + /* * Every regulator should always have coupling descriptor filled with * at least pointer to itself. -- cgit From 3bca239d6184df61a619d78764e0481242d844b4 Mon Sep 17 00:00:00 2001 From: Michał Mirosław Date: Mon, 10 Aug 2020 06:33:32 +0200 Subject: regulator: don't require mutex for regulator_notifier_call_chain() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 3801b86aa482 ("regulator: Refactor supply implementation to work as regular consumers") we no longer cascade notifications and so notifier head's built-in rwsem is enough to protect the notifier chain. Remove the requirement to fix one case where rdev->mutex might be forced to be taken recursively. Signed-off-by: Michał Mirosław Reviewed-by: Dmitry Osipenko Link: https://lore.kernel.org/r/5a0da9017c69a4dbc3f9b50f44476fce80a73387.1597032945.git.mirq-linux@rere.qmqm.pl Signed-off-by: Mark Brown --- drivers/regulator/core.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 07edbd8ebb98..2ed035114ee8 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -4725,14 +4725,11 @@ EXPORT_SYMBOL_GPL(regulator_bulk_free); * @data: callback-specific data. * * Called by regulator drivers to notify clients a regulator event has - * occurred. We also notify regulator clients downstream. - * Note lock must be held by caller. + * occurred. */ int regulator_notifier_call_chain(struct regulator_dev *rdev, unsigned long event, void *data) { - lockdep_assert_held_once(&rdev->mutex.base); - _notifier_call_chain(rdev, event, data); return NOTIFY_DONE; -- cgit From 4c9db39361da5dcf0e77f4aeb4817be3bf7d626b Mon Sep 17 00:00:00 2001 From: Michał Mirosław Date: Sat, 19 Sep 2020 23:28:25 +0200 Subject: regulator: unexport regulator_lock/unlock() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit regulator_lock/unlock() was used only to guard regulator_notifier_call_chain(). As no users remain, make the functions internal. Signed-off-by: Michał Mirosław Link: https://lore.kernel.org/r/d3381aabd2632aff5e7b839d55868bec6e85c811.1600550732.git.mirq-linux@rere.qmqm.pl Signed-off-by: Mark Brown --- drivers/regulator/core.c | 6 ++---- include/linux/regulator/driver.h | 3 --- 2 files changed, 2 insertions(+), 7 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index a9c97c88f50e..6600949d08f8 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -190,11 +190,10 @@ static inline int regulator_lock_nested(struct regulator_dev *rdev, * than the one, which initially locked the mutex, it will * wait on mutex. */ -void regulator_lock(struct regulator_dev *rdev) +static void regulator_lock(struct regulator_dev *rdev) { regulator_lock_nested(rdev, NULL); } -EXPORT_SYMBOL_GPL(regulator_lock); /** * regulator_unlock - unlock a single regulator @@ -203,7 +202,7 @@ EXPORT_SYMBOL_GPL(regulator_lock); * This function unlocks the mutex when the * reference counter reaches 0. */ -void regulator_unlock(struct regulator_dev *rdev) +static void regulator_unlock(struct regulator_dev *rdev) { mutex_lock(®ulator_nesting_mutex); @@ -216,7 +215,6 @@ void regulator_unlock(struct regulator_dev *rdev) mutex_unlock(®ulator_nesting_mutex); } -EXPORT_SYMBOL_GPL(regulator_unlock); static bool regulator_supply_is_couple(struct regulator_dev *rdev) { diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h index 8539f34ae42b..11cade73726c 100644 --- a/include/linux/regulator/driver.h +++ b/include/linux/regulator/driver.h @@ -533,9 +533,6 @@ int regulator_set_current_limit_regmap(struct regulator_dev *rdev, int regulator_get_current_limit_regmap(struct regulator_dev *rdev); void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data); -void regulator_lock(struct regulator_dev *rdev); -void regulator_unlock(struct regulator_dev *rdev); - /* * Helper functions intended to be used by regulator drivers prior registering * their regulators. -- cgit From be35cc4695aa1b26d00b30bfd1d8408eddd357ec Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sun, 20 Sep 2020 15:24:54 +0100 Subject: regulator: fix indentation issue There is a return statement that is indented with an extra space, fix this by removing it. Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20200920142454.33352-1-colin.king@canonical.com Signed-off-by: Mark Brown --- drivers/regulator/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 6600949d08f8..6ef99df1a7d2 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -3065,7 +3065,7 @@ int regulator_get_hardware_vsel_register(struct regulator *regulator, *vsel_reg = rdev->desc->vsel_reg; *vsel_mask = rdev->desc->vsel_mask; - return 0; + return 0; } EXPORT_SYMBOL_GPL(regulator_get_hardware_vsel_register); -- cgit From 99ad5f6ec0cd776e34295ec9fc82bfb004c3fad5 Mon Sep 17 00:00:00 2001 From: Michał Mirosław Date: Sat, 26 Sep 2020 23:32:40 +0200 Subject: regulator: print state at boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the initial state of the regulator shown when debugging. Signed-off-by: Michał Mirosław Link: https://lore.kernel.org/r/53c4f3d394d68f0989174f89e3b0882cebbbd787.1601155770.git.mirq-linux@rere.qmqm.pl Signed-off-by: Mark Brown --- drivers/regulator/core.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 6ef99df1a7d2..42cffbe84080 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1111,10 +1111,15 @@ static void print_constraints(struct regulator_dev *rdev) if (constraints->valid_modes_mask & REGULATOR_MODE_IDLE) count += scnprintf(buf + count, len - count, "idle "); if (constraints->valid_modes_mask & REGULATOR_MODE_STANDBY) - count += scnprintf(buf + count, len - count, "standby"); + count += scnprintf(buf + count, len - count, "standby "); if (!count) - scnprintf(buf, len, "no parameters"); + count = scnprintf(buf, len, "no parameters"); + else + --count; + + count += scnprintf(buf + count, len - count, ", %s", + _regulator_is_enabled(rdev) ? "enabled" : "disabled"); rdev_dbg(rdev, "%s\n", buf); -- cgit From 61aab5ad27d551f1535861e871dd9b03f5fbfd76 Mon Sep 17 00:00:00 2001 From: Michał Mirosław Date: Sat, 26 Sep 2020 23:32:40 +0200 Subject: regulator: print symbolic errors in kernel messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change all error-printing messages to include error name via %pe instead of numeric error or nothing. Signed-off-by: Michał Mirosław Link: https://lore.kernel.org/r/1dcf25f39188882eb56918a9aa281ab17b792aa5.1601155770.git.mirq-linux@rere.qmqm.pl Signed-off-by: Mark Brown --- drivers/regulator/core.c | 94 +++++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 45 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 42cffbe84080..ff8e99ca0306 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -964,7 +964,8 @@ static int drms_uA_update(struct regulator_dev *rdev) /* set the optimum mode for our new total regulator load */ err = rdev->desc->ops->set_load(rdev, current_uA); if (err < 0) - rdev_err(rdev, "failed to set load %d\n", current_uA); + rdev_err(rdev, "failed to set load %d: %pe\n", + current_uA, ERR_PTR(err)); } else { /* get output voltage */ output_uV = regulator_get_voltage_rdev(rdev); @@ -991,14 +992,15 @@ static int drms_uA_update(struct regulator_dev *rdev) /* check the new mode is allowed */ err = regulator_mode_constrain(rdev, &mode); if (err < 0) { - rdev_err(rdev, "failed to get optimum mode @ %d uA %d -> %d uV\n", - current_uA, input_uV, output_uV); + rdev_err(rdev, "failed to get optimum mode @ %d uA %d -> %d uV: %pe\n", + current_uA, input_uV, output_uV, ERR_PTR(err)); return err; } err = rdev->desc->ops->set_mode(rdev, mode); if (err < 0) - rdev_err(rdev, "failed to set optimum mode %x\n", mode); + rdev_err(rdev, "failed to set optimum mode %x: %pe\n", + mode, ERR_PTR(err)); } return err; @@ -1019,14 +1021,14 @@ static int __suspend_set_state(struct regulator_dev *rdev, ret = 0; if (ret < 0) { - rdev_err(rdev, "failed to enabled/disable\n"); + rdev_err(rdev, "failed to enabled/disable: %pe\n", ERR_PTR(ret)); return ret; } if (rdev->desc->ops->set_suspend_voltage && rstate->uV > 0) { ret = rdev->desc->ops->set_suspend_voltage(rdev, rstate->uV); if (ret < 0) { - rdev_err(rdev, "failed to set voltage\n"); + rdev_err(rdev, "failed to set voltage: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1034,7 +1036,7 @@ static int __suspend_set_state(struct regulator_dev *rdev, if (rdev->desc->ops->set_suspend_mode && rstate->mode > 0) { ret = rdev->desc->ops->set_suspend_mode(rdev, rstate->mode); if (ret < 0) { - rdev_err(rdev, "failed to set mode\n"); + rdev_err(rdev, "failed to set mode: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1154,8 +1156,8 @@ static int machine_constraints_voltage(struct regulator_dev *rdev, if (current_uV < 0) { rdev_err(rdev, - "failed to get the current voltage(%d)\n", - current_uV); + "failed to get the current voltage: %pe\n", + ERR_PTR(current_uV)); return current_uV; } @@ -1184,8 +1186,8 @@ static int machine_constraints_voltage(struct regulator_dev *rdev, rdev, target_min, target_max); if (ret < 0) { rdev_err(rdev, - "failed to apply %d-%duV constraint(%d)\n", - target_min, target_max, ret); + "failed to apply %d-%duV constraint: %pe\n", + target_min, target_max, ERR_PTR(ret)); return ret; } } @@ -1334,7 +1336,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, ret = ops->set_input_current_limit(rdev, rdev->constraints->ilim_uA); if (ret < 0) { - rdev_err(rdev, "failed to set input limit\n"); + rdev_err(rdev, "failed to set input limit: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1343,7 +1345,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, if (rdev->constraints->initial_state) { ret = suspend_set_initial_state(rdev); if (ret < 0) { - rdev_err(rdev, "failed to set suspend state\n"); + rdev_err(rdev, "failed to set suspend state: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1356,7 +1358,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, ret = ops->set_mode(rdev, rdev->constraints->initial_mode); if (ret < 0) { - rdev_err(rdev, "failed to set initial mode: %d\n", ret); + rdev_err(rdev, "failed to set initial mode: %pe\n", ERR_PTR(ret)); return ret; } } else if (rdev->constraints->system_load) { @@ -1371,7 +1373,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, && ops->set_ramp_delay) { ret = ops->set_ramp_delay(rdev, rdev->constraints->ramp_delay); if (ret < 0) { - rdev_err(rdev, "failed to set ramp_delay\n"); + rdev_err(rdev, "failed to set ramp_delay: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1379,7 +1381,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, if (rdev->constraints->pull_down && ops->set_pull_down) { ret = ops->set_pull_down(rdev); if (ret < 0) { - rdev_err(rdev, "failed to set pull down\n"); + rdev_err(rdev, "failed to set pull down: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1387,7 +1389,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, if (rdev->constraints->soft_start && ops->set_soft_start) { ret = ops->set_soft_start(rdev); if (ret < 0) { - rdev_err(rdev, "failed to set soft start\n"); + rdev_err(rdev, "failed to set soft start: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1396,7 +1398,8 @@ static int set_machine_constraints(struct regulator_dev *rdev, && ops->set_over_current_protection) { ret = ops->set_over_current_protection(rdev); if (ret < 0) { - rdev_err(rdev, "failed to set over current protection\n"); + rdev_err(rdev, "failed to set over current protection: %pe\n", + ERR_PTR(ret)); return ret; } } @@ -1407,7 +1410,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, ret = ops->set_active_discharge(rdev, ad_state); if (ret < 0) { - rdev_err(rdev, "failed to set active discharge\n"); + rdev_err(rdev, "failed to set active discharge: %pe\n", ERR_PTR(ret)); return ret; } } @@ -1427,7 +1430,7 @@ static int set_machine_constraints(struct regulator_dev *rdev, ret = _regulator_do_enable(rdev); if (ret < 0 && ret != -EINVAL) { - rdev_err(rdev, "failed to enable\n"); + rdev_err(rdev, "failed to enable: %pe\n", ERR_PTR(ret)); return ret; } @@ -1651,8 +1654,8 @@ static struct regulator *create_regulator(struct regulator_dev *rdev, err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj, supply_name); if (err) { - rdev_dbg(rdev, "could not add device link %s err %d\n", - dev->kobj.name, err); + rdev_dbg(rdev, "could not add device link %s: %pe\n", + dev->kobj.name, ERR_PTR(err)); /* non-fatal */ } } @@ -2440,7 +2443,7 @@ static int _regulator_do_enable(struct regulator_dev *rdev) if (ret >= 0) { delay = ret; } else { - rdev_warn(rdev, "enable_time() failed: %d\n", ret); + rdev_warn(rdev, "enable_time() failed: %pe\n", ERR_PTR(ret)); delay = 0; } @@ -2629,7 +2632,7 @@ static int _regulator_enable(struct regulator *regulator) _notifier_call_chain(rdev, REGULATOR_EVENT_ENABLE, NULL); } else if (ret < 0) { - rdev_err(rdev, "is_enabled() failed: %d\n", ret); + rdev_err(rdev, "is_enabled() failed: %pe\n", ERR_PTR(ret)); goto err_consumer_disable; } /* Fallthrough on positive return values - already enabled */ @@ -2731,7 +2734,7 @@ static int _regulator_disable(struct regulator *regulator) ret = _regulator_do_disable(rdev); if (ret < 0) { - rdev_err(rdev, "failed to disable\n"); + rdev_err(rdev, "failed to disable: %pe\n", ERR_PTR(ret)); _notifier_call_chain(rdev, REGULATOR_EVENT_ABORT_DISABLE, NULL); @@ -2798,7 +2801,7 @@ static int _regulator_force_disable(struct regulator_dev *rdev) ret = _regulator_do_disable(rdev); if (ret < 0) { - rdev_err(rdev, "failed to force disable\n"); + rdev_err(rdev, "failed to force disable: %pe\n", ERR_PTR(ret)); _notifier_call_chain(rdev, REGULATOR_EVENT_FORCE_DISABLE | REGULATOR_EVENT_ABORT_DISABLE, NULL); return ret; @@ -2877,7 +2880,8 @@ static void regulator_disable_work(struct work_struct *work) for (i = 0; i < count; i++) { ret = _regulator_disable(regulator); if (ret != 0) - rdev_err(rdev, "Deferred disable failed: %d\n", ret); + rdev_err(rdev, "Deferred disable failed: %pe\n", + ERR_PTR(ret)); } } WARN_ON(!total_count); @@ -3402,7 +3406,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev, } if (delay < 0) { - rdev_warn(rdev, "failed to get delay: %d\n", delay); + rdev_warn(rdev, "failed to get delay: %pe\n", ERR_PTR(delay)); delay = 0; } @@ -3554,8 +3558,8 @@ int regulator_set_voltage_rdev(struct regulator_dev *rdev, int min_uV, ret = regulator_set_voltage_unlocked(rdev->supply, best_supply_uV, INT_MAX, state); if (ret) { - dev_err(&rdev->dev, "Failed to increase supply voltage: %d\n", - ret); + dev_err(&rdev->dev, "Failed to increase supply voltage: %pe\n", + ERR_PTR(ret)); goto out; } } @@ -3572,8 +3576,8 @@ int regulator_set_voltage_rdev(struct regulator_dev *rdev, int min_uV, ret = regulator_set_voltage_unlocked(rdev->supply, best_supply_uV, INT_MAX, state); if (ret) - dev_warn(&rdev->dev, "Failed to decrease supply voltage: %d\n", - ret); + dev_warn(&rdev->dev, "Failed to decrease supply voltage: %pe\n", + ERR_PTR(ret)); /* No need to fail here */ ret = 0; } @@ -4559,8 +4563,8 @@ int regulator_bulk_get(struct device *dev, int num_consumers, err: if (ret != -EPROBE_DEFER) - dev_err(dev, "Failed to get supply '%s': %d\n", - consumers[i].supply, ret); + dev_err(dev, "Failed to get supply '%s': %pe\n", + consumers[i].supply, ERR_PTR(ret)); else dev_dbg(dev, "Failed to get supply '%s', deferring\n", consumers[i].supply); @@ -4618,8 +4622,8 @@ int regulator_bulk_enable(int num_consumers, err: for (i = 0; i < num_consumers; i++) { if (consumers[i].ret < 0) - pr_err("Failed to enable %s: %d\n", consumers[i].supply, - consumers[i].ret); + pr_err("Failed to enable %s: %pe\n", consumers[i].supply, + ERR_PTR(consumers[i].ret)); else regulator_disable(consumers[i].consumer); } @@ -4655,12 +4659,12 @@ int regulator_bulk_disable(int num_consumers, return 0; err: - pr_err("Failed to disable %s: %d\n", consumers[i].supply, ret); + pr_err("Failed to disable %s: %pe\n", consumers[i].supply, ERR_PTR(ret)); for (++i; i < num_consumers; ++i) { r = regulator_enable(consumers[i].consumer); if (r != 0) - pr_err("Failed to re-enable %s: %d\n", - consumers[i].supply, r); + pr_err("Failed to re-enable %s: %pe\n", + consumers[i].supply, ERR_PTR(r)); } return ret; @@ -5043,8 +5047,8 @@ static void regulator_remove_coupling(struct regulator_dev *rdev) if (coupler && coupler->detach_regulator) { err = coupler->detach_regulator(coupler, rdev); if (err) - rdev_err(rdev, "failed to detach from coupler: %d\n", - err); + rdev_err(rdev, "failed to detach from coupler: %pe\n", + ERR_PTR(err)); } kfree(rdev->coupling_desc.coupled_rdevs); @@ -5088,7 +5092,7 @@ static int regulator_init_coupling(struct regulator_dev *rdev) if (IS_ERR(rdev->coupling_desc.coupler)) { err = PTR_ERR(rdev->coupling_desc.coupler); - rdev_err(rdev, "failed to get coupler: %d\n", err); + rdev_err(rdev, "failed to get coupler: %pe\n", ERR_PTR(err)); return err; } @@ -5251,8 +5255,8 @@ regulator_register(const struct regulator_desc *regulator_desc, if (config->ena_gpiod) { ret = regulator_ena_gpio_request(rdev, config); if (ret != 0) { - rdev_err(rdev, "Failed to request enable GPIO: %d\n", - ret); + rdev_err(rdev, "Failed to request enable GPIO: %pe\n", + ERR_PTR(ret)); goto clean; } /* The regulator core took over the GPIO descriptor */ @@ -5837,7 +5841,7 @@ static int regulator_late_cleanup(struct device *dev, void *data) rdev_info(rdev, "disabling\n"); ret = _regulator_do_disable(rdev); if (ret != 0) - rdev_err(rdev, "couldn't disable: %d\n", ret); + rdev_err(rdev, "couldn't disable: %pe\n", ERR_PTR(ret)); } else { /* The intention is that in future we will * assume that full constraints are provided -- cgit From aea6cb99703e17019e025aa71643b4d3e0a24413 Mon Sep 17 00:00:00 2001 From: Michał Mirosław Date: Sat, 26 Sep 2020 23:32:41 +0200 Subject: regulator: resolve supply after creating regulator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When creating a new regulator its supply cannot create the sysfs link because the device is not yet published. Remove early supply resolving since it will be done later anyway. This makes the following error disappear and the symlinks get created instead. DCDC_REG1: supplied by VSYS VSYS: could not add device link regulator.3 err -2 Note: It doesn't fix the problem for bypassed regulators, though. Fixes: 45389c47526d ("regulator: core: Add early supply resolution for regulators") Signed-off-by: Michał Mirosław Link: https://lore.kernel.org/r/ba09e0a8617ffeeb25cb4affffe6f3149319cef8.1601155770.git.mirq-linux@rere.qmqm.pl Signed-off-by: Mark Brown --- drivers/regulator/core.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index ff8e99ca0306..9f704a6c4802 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -5280,15 +5280,20 @@ regulator_register(const struct regulator_desc *regulator_desc, else if (regulator_desc->supply_name) rdev->supply_name = regulator_desc->supply_name; - /* - * Attempt to resolve the regulator supply, if specified, - * but don't return an error if we fail because we will try - * to resolve it again later as more regulators are added. - */ - if (regulator_resolve_supply(rdev)) - rdev_dbg(rdev, "unable to resolve supply\n"); - ret = set_machine_constraints(rdev, constraints); + if (ret == -EPROBE_DEFER) { + /* Regulator might be in bypass mode and so needs its supply + * to set the constraints */ + /* FIXME: this currently triggers a chicken-and-egg problem + * when creating -SUPPLY symlink in sysfs to a regulator + * that is just being created */ + ret = regulator_resolve_supply(rdev); + if (!ret) + ret = set_machine_constraints(rdev, constraints); + else + rdev_dbg(rdev, "unable to resolve supply early: %pe\n", + ERR_PTR(ret)); + } if (ret < 0) goto wash; -- cgit From e9bb4a068b206f61ef01057cfeafdb852fb244c5 Mon Sep 17 00:00:00 2001 From: AngeloGioacchino Del Regno Date: Sat, 26 Sep 2020 14:55:43 +0200 Subject: regulator: core: Enlarge max OF property name length to 64 chars Some regulator drivers may be defining very long names: this is the case with the qcom_smd and qcom_spmi regulators, where we need to parse the regulator parents from DT. For clarity, this is an example: { "l13a", QCOM_SMD_RPM_LDOA, 13, &pm660_ht_lvpldo, "vdd_l8_l9_l10_l11_l12_l13_l14" }, pm660-regulators { ... vdd_l8_l9_l10_l11_l12_l13_l14-supply = <&vreg_s4a_2p04> ... }; Now, with a 32 characters limit, the function is trying to parse, exactly, "vdd_l8_l9_l10_l11_l12_l13_l14-s" (32 chars) instead of the right one, which is 37 chars long in this specific case. ... And this is not only the case with PM660/PM660L, but also with PMA8084, PM8916, PM8950 and others that are not implemented yet. The length of 64 chars was chosen based on the longest parsed property name that I could find, which is in PM8916, and would be 53 characters long. At that point, rounding that to 64 looked like being the best idea. Signed-off-by: AngeloGioacchino Del Regno Link: https://lore.kernel.org/r/20200926125549.13191-2-kholk11@gmail.com Signed-off-by: Mark Brown --- drivers/regulator/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 9f704a6c4802..16497192e99b 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -404,11 +404,11 @@ err_node_put: static struct device_node *of_get_regulator(struct device *dev, const char *supply) { struct device_node *regnode = NULL; - char prop_name[32]; /* 32 is max size of property name */ + char prop_name[64]; /* 64 is max size of property name */ dev_dbg(dev, "Looking up %s-supply from device tree\n", supply); - snprintf(prop_name, 32, "%s-supply", supply); + snprintf(prop_name, 64, "%s-supply", supply); regnode = of_parse_phandle(dev->of_node, prop_name, 0); if (!regnode) { -- cgit From c845f21ad86528f888db27849f2d315e08126816 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 5 Oct 2020 15:15:46 +0200 Subject: regulator: Make constraint debug processing conditional on DEBUG If debugging is disabled, print_constraints() does not print the actual constraints, but still performs some processing and string formatting, only to throw away the result later. Fix this by moving all constraint debug processing to a separate function, and replacing it by a dummy when debugging is disabled. This reduces kernel size by almost 800 bytes (on arm/arm64). Signed-off-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/20201005131546.22448-1-geert+renesas@glider.be Signed-off-by: Mark Brown --- drivers/regulator/core.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'drivers/regulator/core.c') diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 16497192e99b..bba25935e79e 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1056,7 +1056,8 @@ static int suspend_set_initial_state(struct regulator_dev *rdev) return __suspend_set_state(rdev, rstate); } -static void print_constraints(struct regulator_dev *rdev) +#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +static void print_constraints_debug(struct regulator_dev *rdev) { struct regulation_constraints *constraints = rdev->constraints; char buf[160] = ""; @@ -1124,6 +1125,16 @@ static void print_constraints(struct regulator_dev *rdev) _regulator_is_enabled(rdev) ? "enabled" : "disabled"); rdev_dbg(rdev, "%s\n", buf); +} +#else /* !DEBUG && !CONFIG_DYNAMIC_DEBUG */ +static inline void print_constraints_debug(struct regulator_dev *rdev) {} +#endif /* !DEBUG && !CONFIG_DYNAMIC_DEBUG */ + +static void print_constraints(struct regulator_dev *rdev) +{ + struct regulation_constraints *constraints = rdev->constraints; + + print_constraints_debug(rdev); if ((constraints->min_uV != constraints->max_uV) && !regulator_ops_is_valid(rdev, REGULATOR_CHANGE_VOLTAGE)) -- cgit