2017-06-27 06:06:59

by Gaurav Gupta

Subject: [PATCH] regulator: core: Lookup based on supply name alone as last option

If there are two regulators with the same supply name, the current
lookup just based on the supply name would always return the first one.
Further in the function, there is code to check both dev name and supply
name and return the matching regulator. This should be done before the
lookup based on supply name alone, which should be a last resort.

Signed-off-by: Gaurav Gupta <[email protected]>
---
drivers/regulator/core.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c0d9ae8..80460df 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1489,10 +1489,6 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
if (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) {
/* If the mapping has a device set up it must match */
@@ -1508,6 +1504,7 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
}
mutex_unlock(&regulator_list_mutex);

+ r = regulator_lookup_by_name(supply);
if (r)
return r;

--
2.10.2.dirty

2017-06-27 16:28:43

by Charles Keepax

Subject: Re: [PATCH] regulator: core: Lookup based on supply name alone as last option

On Mon, Jun 26, 2017 at 10:57:12PM -0700, Gaurav Gupta wrote:
> If there are two regulators with the same supply name, the current
> lookup just based on the supply name would always return the first one.
> Further in the function, there is code to check both dev name and supply
> name and return the matching regulator. This should be done before the
> lookup based on supply name alone, which should be a last resort.
>
> Signed-off-by: Gaurav Gupta <[email protected]>
> ---

A patch has already been merged to do this:

https://lkml.org/lkml/2017/6/13/852

Thanks,
Charles

> drivers/regulator/core.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index c0d9ae8..80460df 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -1489,10 +1489,6 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
> if (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) {
> /* If the mapping has a device set up it must match */
> @@ -1508,6 +1504,7 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
> }
> mutex_unlock(&regulator_list_mutex);
>
> + r = regulator_lookup_by_name(supply);

Also the loop above doesn't return it simply sets r so this would
overwrite the discovered regulator with the lookup based on
supply name alone.

> if (r)
> return r;
>
> --
> 2.10.2.dirty