Re: [PATCH] regulator: core: Don't link consumers on the same device

[Marek Szyprowski]

Hi Mark,

On 2018-07-09 14:54, Mark Brown wrote:
> In some cases a device may end up supplying itself, for example when a
> DCDC is used to improve the efficiency of LDOs or when a LDO is provided
> to clean up sensitive supplies.  In these cases the driver core will
> warn loudly about attempts to add links so suppress them here.
>
> Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
> ---
>
> Compile tested only.

This patch doesn't help, because wm8994 creates separate struct device for
each registered regulator, so you need to check the regulator parents too,
see my fixes below in the patch.

I don't like such fixes for this specific case. It's not that uncommon
that a complex device provides various functionalities used by its
components, so it is not a bad design to use regulator provided by
its parent/grandparent/other ancestor device.

Frankly, either the device_is_dependent() function should be exported and
used in regulator core or device_link_add() should have another flag:
DL_FLAG_SKIP_DEPENDANT.

>   drivers/regulator/core.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index b5db0257bc40..1302e66e80ac 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -1741,7 +1741,8 @@ struct regulator *_regulator_get(struct device *dev, const char *id,
>   			rdev->use_count = 0;
>   	}
>   
> -	device_link_add(dev, &rdev->dev, DL_FLAG_STATELESS);
> +	if (dev != &rdev->dev)

 ÂÂ if (dev != &rdev->dev && dev != rdev->dev.parent)

> +		device_link_add(dev, &rdev->dev, DL_FLAG_STATELESS);
>   
>   	return regulator;
>   }
> @@ -1840,7 +1841,7 @@ static void _regulator_put(struct regulator *regulator)
>   			if (r->dev == regulator->dev)
>   				count++;
>   
> -		if (count == 1)
> +		if (count == 1 && regulator->dev != &rdev->dev)

 ÂÂ if (count == 1 && regulator->dev != &rdev->dev &&
 ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ regulator->dev != rdev->dev.parent)

>   			device_link_remove(regulator->dev, &rdev->dev);
>   
>   		/* remove any sysfs entries */

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland