[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJOA=zNtKvvek5LPsDYUie8KLcTBB0F4GxWhzn2g8=YxpCcMPQ@mail.gmail.com>
Date: Fri, 13 Jan 2012 20:39:07 -0800
From: "Turquette, Mike" <mturquette@...com>
To: Saravana Kannan <skannan@...eaurora.org>
Cc: Russell King - ARM Linux <linux@....linux.org.uk>, andrew@...n.ch,
linaro-dev@...ts.linaro.org, eric.miao@...aro.org,
grant.likely@...retlab.ca, Colin Cross <ccross@...gle.com>,
jeremy.kerr@...onical.com, sboyd@...cinc.com,
magnus.damm@...il.com, dsaxena@...aro.org,
linux-arm-kernel@...ts.infradead.org, arnd.bergmann@...aro.org,
patches@...aro.org, Thomas Gleixner <tglx@...utronix.de>,
linux-omap@...r.kernel.org, richard.zhao@...aro.org,
shawn.guo@...escale.com, paul@...an.com,
linus.walleij@...ricsson.com, broonie@...nsource.wolfsonmicro.com,
linux-kernel@...r.kernel.org, amit.kucheria@...aro.org,
skannan@...cinc.com
Subject: Re: [PATCH v4 3/6] clk: introduce the common clock framework
On Fri, Jan 13, 2012 at 8:18 PM, Saravana Kannan <skannan@...eaurora.org> wrote:
> On 12/17/2011 03:04 AM, Russell King - ARM Linux wrote:
>>
>> On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
>>>
>>> On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner<tglx@...utronix.de>
>>> wrote:
>>>>
>>>> On Tue, 13 Dec 2011, Mike Turquette wrote:
>>>>>
>>>>> +void __clk_unprepare(struct clk *clk)
>>>>> +{
>>>>> + if (!clk)
>>>>> + return;
>>>>> +
>>>>> + if (WARN_ON(clk->prepare_count == 0))
>>>>> + return;
>>>>> +
>>>>> + if (--clk->prepare_count> 0)
>>>>> + return;
>>>>> +
>>>>> + WARN_ON(clk->enable_count> 0);
>>>>
>>>>
>>>> So this leaves the clock enable count set. I'm a bit wary about
>>>> that. Shouldn't it either return (including bumping the prepare_count
>>>> again) or call clk_disable() ?
>>
>>
>> No it should not.
>>
>>> I've hit this in my port of OMAP. It comes from this simple situation:
>>>
>>> driver 1 (adapted for clk_prepare/clk_unprepare):
>>> clk_prepare(clk);
>>> clk_enable(clk);
>>>
>>> ...
>>>
>>> driver2 (not adapted for clk_prepare/clk_unprepare):
>>> clk_enable(clk);
>>
>>
>> So this is basically buggy. Look, it's quite simple. Convert _all_
>> your drivers to clk_prepare/clk_unprepare _before_ you start switching
>> your platform to use these new functions. You can do that _today_
>> without exception.
>>
>> We must refuse to merge _any_ user which does this the old way - and
>> we should have been doing this since my commit was merged into mainline
>> to allow drivers to be converted.
>>
>> And stop trying to think of ways around this inside clk_prepare/
>> clk_unprepare/clk_enable/clk_disable. You can't do it. Just fix _all_
>> the drivers. Now. Before you start implementing
>> clk_prepare/clk_unprepare.
>
>
> I agree with Russell's suggestion. This is what I'm trying to do with the
> MSM platform. Not sure if I'm too optimistic, but as of today, I'm still
> optimistic I can push the MSM driver devs to get this done before we enable
> real prepare/unprepare support.
Just to reach closure on this topic: I don't plan to change
__clk_unprepare in the next version of the patches. The warnings are
doing a fine job of catching code which has yet to be properly
converted to use clk_(un)prepare.
Mike
>
>
> Thanks,
> Saravana
>
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists