[PATCH] regulator: core: Prioritise consumer mappings over regulator name

[PATCH] regulator: core: Prioritise consumer mappings over regulator name

[Charles Keepax]

Currently, when looking up a regulator supply, the regulator name
takes priority over the consumer mappings. As there are a lot of
regulator names that are in fairly common use (VDD, MICVDD, etc.) this
can easily lead to obtaining the wrong supply, when a system contains
two regulators that share a name.

The explicit consumer mappings contain much less ambiguity as they
specify both a name and a consumer device. As such prioritise those if
one exists and only fall back to the regulator name if there are no
matching explicit mappings.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 919b7f1..d257952 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1490,8 +1490,6 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 		devname = dev_name(dev);
 
 	r = regulator_lookup_by_name(supply);
-	if (r)
-		return r;
 
 	mutex_lock(&regulator_list_mutex);
 	list_for_each_entry(map, &regulator_map_list, list) {
-- 
2.1.4

Re: [PATCH] regulator: core: Prioritise consumer mappings over regulator name

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 169 bytes --]

On Mon, Jun 12, 2017 at 04:17:52PM +0100, Charles Keepax wrote:

>  	r = regulator_lookup_by_name(supply);
> -	if (r)
> -		return r;

Why have you left the lookup here?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regulator: core: Prioritise consumer mappings over regulator name

[Charles Keepax]

On Mon, Jun 12, 2017 at 05:38:56PM +0100, Mark Brown wrote:
> On Mon, Jun 12, 2017 at 04:17:52PM +0100, Charles Keepax wrote:
> 
> >  	r = regulator_lookup_by_name(supply);
> > -	if (r)
> > -		return r;
> 
> Why have you left the lookup here?

Yeah was thinking that could maybe use a comment, if we don't
find a match in the following loop over the supply map then we
will exit with the regulator found here. So we can still use the
regulator name for lookup just we default to the supply map.

Thanks,
Charles

Re: [PATCH] regulator: core: Prioritise consumer mappings over regulator name

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 669 bytes --]

On Tue, Jun 13, 2017 at 09:07:31AM +0100, Charles Keepax wrote:
> On Mon, Jun 12, 2017 at 05:38:56PM +0100, Mark Brown wrote:
> > On Mon, Jun 12, 2017 at 04:17:52PM +0100, Charles Keepax wrote:

> > >  	r = regulator_lookup_by_name(supply);
> > > -	if (r)
> > > -		return r;

> > Why have you left the lookup here?

> Yeah was thinking that could maybe use a comment, if we don't
> find a match in the following loop over the supply map then we
> will exit with the regulator found here. So we can still use the
> regulator name for lookup just we default to the supply map.

Why are we even doing the lookup here if we only use it if we fail to
find a supply mapping?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regulator: core: Prioritise consumer mappings over regulator name

[Charles Keepax]

On Tue, Jun 13, 2017 at 11:15:56AM +0100, Mark Brown wrote:
> On Tue, Jun 13, 2017 at 09:07:31AM +0100, Charles Keepax wrote:
> > On Mon, Jun 12, 2017 at 05:38:56PM +0100, Mark Brown wrote:
> > > On Mon, Jun 12, 2017 at 04:17:52PM +0100, Charles Keepax wrote:
> 
> > > >  	r = regulator_lookup_by_name(supply);
> > > > -	if (r)
> > > > -		return r;
> 
> > > Why have you left the lookup here?
> 
> > Yeah was thinking that could maybe use a comment, if we don't
> > find a match in the following loop over the supply map then we
> > will exit with the regulator found here. So we can still use the
> > regulator name for lookup just we default to the supply map.
> 
> Why are we even doing the lookup here if we only use it if we fail to
> find a supply mapping?

Yeah I think that is probably a poor choice I will do a V2 that
does the lookup conditionally after the search of the supply map.

Thanks,
Charles