| Message ID | CALAqxLWKg8WoSAi7zgvJ4vTPqhK1fU+q-q+-QZOEB+mUZrdOuA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 2, 2015 at 2:03 PM, John Stultz <john.stultz@linaro.org> wrote: > On Mon, Nov 2, 2015 at 12:46 PM, Bjorn Andersson > <bjorn.andersson@sonymobile.com> wrote: >> On Mon 02 Nov 11:57 PST 2015, John Stultz wrote: >> >>> Trying to move my nexus7 work to -next, I started seeing boot time >>> hangs. Enabling some debug options provided with a lockdep spew. >>> >>> Reverting "regulator: core: Propagate voltage changes to supply >>> regulators" - fc42112c0eaa avoids the hang, but I still see lockdep >>> noise. >>> >>> Full log of -next based tree without any reverts follows: >> >> It sure does lock like a deadlock to me. Can you figure out which >> regulator we're talking about here? And confirm that this is 8064 we're >> talking about > > So it looks like we're setting the voltage on lvs1, which then calls > get_voltage() on s4. At that point the lockdep spew hits. > > But as Mark noted *this* spew looks like a false positive, but I > suspect lockdep is then disabled and can't detect the real hang > happens shortly afterwards. Further on this, the lockdep spew happens w/ v4.3 as well. Its just the deadlock in -next got me to add lockdep in my config. thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Mon, Nov 02, 2015 at 02:03:14PM -0800, John Stultz wrote: > Might the problem be here that we lock the supply in set_voltage, then > if we call _regulator_get_voltage on the supply later, we try to grab > the same lock and we're stuck? No, the internal get voltage call shouldn't be locking in the first place (and indeed it doesn't do so AFAICT?).
On Mon, Nov 2, 2015 at 2:58 PM, Mark Brown <broonie@kernel.org> wrote: > On Mon, Nov 02, 2015 at 02:03:14PM -0800, John Stultz wrote: > >> Might the problem be here that we lock the supply in set_voltage, then >> if we call _regulator_get_voltage on the supply later, we try to grab >> the same lock and we're stuck? > > No, the internal get voltage call shouldn't be locking in the first > place (and indeed it doesn't do so AFAICT?). drivers/regulator/core.c: @3063 static int _regulator_get_voltage(struct regulator_dev *rdev) { int sel, ret; if (rdev->desc->ops->get_voltage_sel) { ... } else if (rdev->supply) { ret = regulator_get_voltage(rdev->supply); <----- } else { Where _regulator_get_voltage() is called from regulator_set_voltage_unlocked(), called from regulator_set_voltage(). thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Mon, Nov 02, 2015 at 03:04:29PM -0800, John Stultz wrote: > On Mon, Nov 2, 2015 at 2:58 PM, Mark Brown <broonie@kernel.org> wrote: > > No, the internal get voltage call shouldn't be locking in the first > > place (and indeed it doesn't do so AFAICT?). > } else if (rdev->supply) { > ret = regulator_get_voltage(rdev->supply); <----- > } else { > Where _regulator_get_voltage() is called from > regulator_set_voltage_unlocked(), called from regulator_set_voltage(). > Well, that's the issue then - get_voltage() needs to be locking the supplies like set_voltage() does.
--- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -2952,11 +2952,12 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV) { int ret = 0; - regulator_lock_supply(regulator->rdev); + mutex_lock(®ulator->rdev->mutex); ret = regulator_set_voltage_unlocked(regulator, min_uV, max_uV); - regulator_unlock_supply(regulator->rdev); + mutex_unlock(®ulator->rdev->mutex); return ret; }