Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756158AbbGVGqO (ORCPT <rfc822;w@1wt.eu>);
	Wed, 22 Jul 2015 02:46:14 -0400
Received: from mailout1.w1.samsung.com ([210.118.77.11]:46357 "EHLO
	mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752434AbbGVGqK (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 22 Jul 2015 02:46:10 -0400
X-AuditID: cbfec7f5-f794b6d000001495-49-55af3c2f5248
Message-id: <55AF3C2E.7010205@samsung.com>
Date: Wed, 22 Jul 2015 15:46:06 +0900
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101
 Thunderbird/31.8.0
MIME-version: 1.0
To: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>,
        Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-arm-kernel@lists.infradead.org, robh+dt@kernel.org,
        mturquette@baylibre.com, lee.jones@linaro.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-clk@vger.kernel.org
Subject: Re: [PATCH 3/4] clk: 88pm800: Add clk provider driver for 88pm800
 family of devices
References: <1437476823-3358-1-git-send-email-vaibhav.hiremath@linaro.org>
 <1437476823-3358-4-git-send-email-vaibhav.hiremath@linaro.org>
 <55AE990E.2040004@codeaurora.org> <55AE9F48.20600@linaro.org>
 <55AEB103.9010205@codeaurora.org> <55AF37D9.3000006@linaro.org>
In-reply-to: <55AF37D9.3000006@linaro.org>
Content-type: text/plain; charset=windows-1252
Content-transfer-encoding: 7bit
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrALMWRmVeSWpSXmKPExsVy+t/xK7r6NutDDc5/0raYf+Qcq8XrF4YW
	978eZbTY9Pgaq8XHnnusFpd3zWGzuHjK1aJ17xF2ix9nulksrl/dyOLA5fH+Riu7x+W+XiaP
	Tas62TzuXNvD5rF5Sb1H35ZVjB6fN8kFsEdx2aSk5mSWpRbp2yVwZVw78Iu54KhgxbEVBg2M
	L3m7GDk5JARMJF69WMMEYYtJXLi3nq2LkYtDSGApo8TLQ5MYIZwvjBIvDl9lBaniFdCSeHRh
	OjOIzSKgKrGy9wZYN5uAscTm5UvYQGxRgQiJ5atPMkLUC0r8mHyPBcQWEYiU+LyviQVkKLPA
	EUaJrSsegyWEBeIkDrUth9rWyySxbe0SoAQHByfQtpn7nUBMZgE9ifsXtUDKmQXkJTavecs8
	gVFgFpIVsxCqZiGpWsDIvIpRNLU0uaA4KT3XSK84Mbe4NC9dLzk/dxMjJB6+7mBceszqEKMA
	B6MSD++Eo+tChVgTy4orcw8xSnAwK4nwKnOvDxXiTUmsrEotyo8vKs1JLT7EKM3BoiTOO3PX
	+xAhgfTEktTs1NSC1CKYLBMHp1QDo/Wb2JDer5sifjQzXWqrCPwazrZNrX3m9IK2mdya8oeU
	pXS+f8vzfRfvp6XS+zEoj927fbFH9r20+c8lmG7N4NX2P82XrLdogkH931kv/LSsLt9dsG3t
	T32GmwKHph+PqfLuu/Gn0u37TBnRD7zF4h/rtCPPVW413yCfuHePN1eHYt+nV/MfK7EUZyQa
	ajEXFScCAD0Ox0WDAgAA
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2419
Lines: 76

On 22.07.2015 15:27, Vaibhav Hiremath wrote:
> 
> 
> On Wednesday 22 July 2015 02:22 AM, Stephen Boyd wrote:
>> On 07/21/2015 12:36 PM, Vaibhav Hiremath wrote:
>>>
>>> On Wednesday 22 July 2015 12:40 AM, Stephen Boyd wrote:
>>>> On 07/21/2015 04:07 AM, Vaibhav Hiremath wrote:
>>>>> +
> 
> <snip>
> 
>>>>> +static int pm800_clk_remove(struct platform_device *pdev)
>>>>> +{
>>>>> +    struct pm800_clk *pm800_clks = platform_get_drvdata(pdev);
>>>>> +    int i;
>>>>> +
>>>>> +    of_clk_del_provider(pm800_clks[0].clk_np);
>>>>> +    /* Drop the reference obtained in pm800_clk_parse_dt */
>>>>> +    of_node_put(pm800_clks[0].clk_np);
>>>>
>>>> This is odd. Why are we keeping the reference in the driver?
>>>>
>>>
>>> Honestly I do not have any good answer here. I have to admit that it is
>>> getting carry forwarded from legacy driver.
>>>
>>
>> Well we shouldn't do things if we don't know why we're doing them.
>> Krzysztof?

I am really busy now so I am not following closely other discussions. I
assume you are referring to clk-s2mps11.c. The of_node_put() matches
of_get_child_by_name() when parsing DT.

So why not of_node_put() just after parsing DT? Well, the result of
of_get_child_by_name() is stored in state container for entire device
life-cycle so we can use it in of_clk_del_provider().

That was the idea behind it. If it looks incorrect I would be happy to
see a patch :) .

>>
> 
> Hold on,
> After looking more in to this, it seems we really do not need it.
> It is already taken care by
> 
> of_clk_add_provider() and
> of_clk_del_provider()
> 
> Sorry for not investigating this before. Just left out from my eyes
> somehow.
> 
> 
> Actually I can cleanup clk-s2mps11.c driver as well, but only thing is
> I can validate it, as I do not have platform to test it.
> It should be trivial changes.
> 
> If somebody can help me out in validation I can submit the patch for
> clk-s2mps11.c driver as well.

Sure, I can do this. The clock is present on few devices I can test.
Depending on the current workqueue it may take few days. Just please
mark the patch RFT so it won't be applied before receiving
reviewed/tested tags.

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/