Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] regulator: core: Don't link consumers on the same
 device
To:     Mark Brown <broonie@kernel.org>, pascal paillet <p.paillet@st.com>,
        benjamin.gaignard@linaro.org
Cc:     Krzysztof Kozlowski <krzk@kernel.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        linux-kernel@vger.kernel.org
From:   Marek Szyprowski <m.szyprowski@samsung.com>
Date:   Mon, 9 Jul 2018 15:32:14 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
        Thunderbird/52.9.0
MIME-Version: 1.0
In-Reply-To: <20180709125402.32427-1-broonie@kernel.org>
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Message-Id: <20180709133215eucas1p138cd9714d802a446973ce90058ef0afb~-tpG-E94E1650416504eucas1p1I@eucas1p1.samsung.com>
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <CGME20180709125425epcas1p2343265d3a6a826b2ace0ef584d062dac@epcas1p2.samsung.com>
        <20180709125402.32427-1-broonie@kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@samsung.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>
> 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