Commit 4272f954 authored by Doug Anderson's avatar Doug Anderson Committed by Luis Henriques

regulator: core: Fix enable GPIO reference counting

commit 29d62ec5 upstream.

Normally _regulator_do_enable() isn't called on an already-enabled
rdev.  That's because the main caller, _regulator_enable() always
calls _regulator_is_enabled() and only calls _regulator_do_enable() if
the rdev was not already enabled.

However, there is one caller of _regulator_do_enable() that doesn't
check: regulator_suspend_finish().  While we might want to make
regulator_suspend_finish() behave more like _regulator_enable(), it's
probably also a good idea to make _regulator_do_enable() robust if it
is called on an already enabled rdev.

At the moment, _regulator_do_enable() is _not_ robust for already
enabled rdevs if we're using an ena_pin.  Each time
_regulator_do_enable() is called for an rdev using an ena_pin the
reference count of the ena_pin is incremented even if the rdev was
already enabled.  This is not as intended because the ena_pin is for
something else: for keeping track of how many active rdevs there are
sharing the same ena_pin.

Here's how the reference counting works here:

* Each time _regulator_enable() is called we increment
  rdev->use_count, so _regulator_enable() calls need to be balanced
  with _regulator_disable() calls.

* There is no explicit reference counting in _regulator_do_enable()
  which is normally just a warapper around rdev->desc->ops->enable()
  with code for supporting delays.  It's not expected that the
  "ops->enable()" call do reference counting.

* Since regulator_ena_gpio_ctrl() does have reference counting
  (handling the sharing of the pin amongst multiple rdevs), we
  shouldn't call it if the current rdev is already enabled.

Note that as part of this we cleanup (remove) the initting of
ena_gpio_state in regulator_register().  In _regulator_do_enable(),
_regulator_do_disable() and _regulator_is_enabled() is is clear that
ena_gpio_state should be the state of whether this particular rdev has
requested the GPIO be enabled.  regulator_register() was initting it
as the actual state of the pin.

Fixes: 967cfb18 ("regulator: core: manage enable GPIO list")
Signed-off-by: default avatarDoug Anderson <dianders@chromium.org>
Signed-off-by: default avatarMark Brown <broonie@kernel.org>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent c6d1fec4
...@@ -1771,10 +1771,12 @@ static int _regulator_do_enable(struct regulator_dev *rdev) ...@@ -1771,10 +1771,12 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
trace_regulator_enable(rdev_get_name(rdev)); trace_regulator_enable(rdev_get_name(rdev));
if (rdev->ena_pin) { if (rdev->ena_pin) {
ret = regulator_ena_gpio_ctrl(rdev, true); if (!rdev->ena_gpio_state) {
if (ret < 0) ret = regulator_ena_gpio_ctrl(rdev, true);
return ret; if (ret < 0)
rdev->ena_gpio_state = 1; return ret;
rdev->ena_gpio_state = 1;
}
} else if (rdev->desc->ops->enable) { } else if (rdev->desc->ops->enable) {
ret = rdev->desc->ops->enable(rdev); ret = rdev->desc->ops->enable(rdev);
if (ret < 0) if (ret < 0)
...@@ -1904,10 +1906,12 @@ static int _regulator_do_disable(struct regulator_dev *rdev) ...@@ -1904,10 +1906,12 @@ static int _regulator_do_disable(struct regulator_dev *rdev)
trace_regulator_disable(rdev_get_name(rdev)); trace_regulator_disable(rdev_get_name(rdev));
if (rdev->ena_pin) { if (rdev->ena_pin) {
ret = regulator_ena_gpio_ctrl(rdev, false); if (rdev->ena_gpio_state) {
if (ret < 0) ret = regulator_ena_gpio_ctrl(rdev, false);
return ret; if (ret < 0)
rdev->ena_gpio_state = 0; return ret;
rdev->ena_gpio_state = 0;
}
} else if (rdev->desc->ops->disable) { } else if (rdev->desc->ops->disable) {
ret = rdev->desc->ops->disable(rdev); ret = rdev->desc->ops->disable(rdev);
...@@ -3479,12 +3483,6 @@ regulator_register(const struct regulator_desc *regulator_desc, ...@@ -3479,12 +3483,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
config->ena_gpio, ret); config->ena_gpio, ret);
goto wash; goto wash;
} }
if (config->ena_gpio_flags & GPIOF_OUT_INIT_HIGH)
rdev->ena_gpio_state = 1;
if (config->ena_gpio_invert)
rdev->ena_gpio_state = !rdev->ena_gpio_state;
} }
/* set regulator constraints */ /* set regulator constraints */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment