Re: Deadlock scenario in regulator core

[Mark Brown]

On Tue, Mar 22, 2011 at 04:30:45PM -0700, David Collins wrote:
> On 03/22/2011 03:31 PM, Mark Brown wrote:

> > +	int disret;
> > +
> > +	if (rdev->supply) {
> > +		ret = regulator_enable(rdev->supply);

> This should be _regulator_enable instead of regulator_enable.  There will

Oh, feh.  The supply stuff would generally be easier if it were
consumers as you'd expect, the special casing just makes things more
fragile.  It's not clear to me that the best approach here isn't just to
make the supplies have regular consumer structs so we can do things like
this.

> also need to be a mutex lock and unlock around it for rdev->supply->mutex.

Unless we implement the above change - you're assuming that the change
to the unlocked enable is the best one.

>  I think that it needs to iterate through all supplies in the chain
> similar to how it is done in regulator_disable.

The current code (if it had compiled) would deal with that through
recursion.

> This should be _regulator_disable instead of regulator_disable.  There
> will also need to be a mutex lock and unlock around it for
> rdev->supply->mutex.  Additionally, a while loop is needed to disable all
> supplies in the chain (same as in regulator_disable).

Again, no loop needed with the code as written as it's recursive.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/