From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754755Ab2ANEjb (ORCPT ); Fri, 13 Jan 2012 23:39:31 -0500 Received: from na3sys009aog124.obsmtp.com ([74.125.149.151]:55479 "EHLO na3sys009aog124.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754388Ab2ANEj3 convert rfc822-to-8bit (ORCPT ); Fri, 13 Jan 2012 23:39:29 -0500 MIME-Version: 1.0 In-Reply-To: <4F11021A.8070407@codeaurora.org> References: <1323834838-2206-1-git-send-email-mturquette@linaro.org> <1323834838-2206-4-git-send-email-mturquette@linaro.org> <20111217110433.GC14547@n2100.arm.linux.org.uk> <4F11021A.8070407@codeaurora.org> From: "Turquette, Mike" Date: Fri, 13 Jan 2012 20:39:07 -0800 Message-ID: Subject: Re: [PATCH v4 3/6] clk: introduce the common clock framework To: Saravana Kannan Cc: Russell King - ARM Linux , andrew@lunn.ch, linaro-dev@lists.linaro.org, eric.miao@linaro.org, grant.likely@secretlab.ca, Colin Cross , jeremy.kerr@canonical.com, sboyd@quicinc.com, magnus.damm@gmail.com, dsaxena@linaro.org, linux-arm-kernel@lists.infradead.org, arnd.bergmann@linaro.org, patches@linaro.org, Thomas Gleixner , linux-omap@vger.kernel.org, richard.zhao@linaro.org, shawn.guo@freescale.com, paul@pwsan.com, linus.walleij@stericsson.com, broonie@opensource.wolfsonmicro.com, linux-kernel@vger.kernel.org, amit.kucheria@linaro.org, skannan@quicinc.com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 13, 2012 at 8:18 PM, Saravana Kannan 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 >>>  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.