2017-06-12 15:16:43

by Charles Keepax

[permalink] [raw]
Subject: [PATCH] regulator: core: Prioritise consumer mappings over regulator name

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 <[email protected]>
---
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


2017-06-12 16:39:28

by Mark Brown

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

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?


Attachments:
(No filename) (169.00 B)
signature.asc (488.00 B)
Download all attachments

2017-06-13 08:06:23

by Charles Keepax

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

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

2017-06-13 10:16:07

by Mark Brown

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

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?


Attachments:
(No filename) (669.00 B)
signature.asc (488.00 B)
Download all attachments

2017-06-13 13:15:12

by Charles Keepax

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

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