During regulator registration the regulator framework checks that the
regulator is:
a) enabled
b) has a supplier
and if both are true, it calls regulator_enable() on the supplier, which
has the side effect of incrementing its use count. This puts the
regulator framework in an inconsistent state. Consider the following
example:
regY --supplies--> regX
regX is turned on by hardware, bootloader, or driver ops (as a result of
the boot_on or always_on flag being set), so during registration both
(a) and (b) are true. regulator_enable(regY) is invoked, and now
regY.use_count == 1, regX.use_count == 0.
Later on, the regulator framework iterates over the registered
regulators and turns off unused ones using driver ops. Due to the
use_count values, regX is disabled, while regY is not. Due to the use of
driver ops, instead of regulator_disable(), the use_count of regY is not
updated when regX is disabled.
We now have a situation where regY has a minimum use_count of 1, and
will never be turned off.
The underlying issue is that we are updating regulator metadata that
exists to keep track of function invocations (regulator_enable,
regulator_disable) as a result of something that isn't said function
invocations. The state of regX is not the result of a regulator_enable
call, so use_count increments should not be propogated on the supply
dependency chain. Instead, if there's a need for regX to be enabled on
boot, the boot_on flag should be used for all the regulators on the
supply chain.
Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
Signed-off-by: Nikita Kiryanov <[email protected]>
Signed-off-by: Igor Grinberg <[email protected]>
---
drivers/regulator/core.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a01b8b3..7070d5c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3440,13 +3440,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
ret = set_supply(rdev, r);
if (ret < 0)
goto scrub;
-
- /* Enable supply if rail is enabled */
- if (_regulator_is_enabled(rdev)) {
- ret = regulator_enable(rdev->supply);
- if (ret < 0)
- goto scrub;
- }
}
add_dev:
--
1.8.1.2
On Sun, Oct 13, 2013 at 06:01:11PM +0300, Nikita Kiryanov wrote:
> During regulator registration the regulator framework checks that the
> regulator is:
Please try to make your commit messages briefer and more focused on
describing the problem. A good way to do this is usually to start from
the problem and then explain it - what you're doing here is providing a
lot of background material without telling the reader why which makes it
hard to find out what the commit is supposed to do.
> invocations. The state of regX is not the result of a regulator_enable
> call, so use_count increments should not be propogated on the supply
> dependency chain. Instead, if there's a need for regX to be enabled on
> boot, the boot_on flag should be used for all the regulators on the
> supply chain.
No, that's not what this is trying to do. It's trying to bring the
logical state of the system in sync with the physical state of the
system (as you'll see from the commit introducing it). I suspect what
needs to happen to maintain the functionality is that the code needs to
directly call the ops (and walk the tree manually) to do the enables so
that the reference counts aren't disturbed.
boot_on definitely shouldn't be related to this and probably shouldn't
need all the parent regulators to be manually forced on either.