From patchwork Thu Mar 15 00:51:48 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mike Turquette X-Patchwork-Id: 7300 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id C7EB323DEA for ; Thu, 15 Mar 2012 00:52:12 +0000 (UTC) Received: from mail-iy0-f180.google.com (mail-iy0-f180.google.com [209.85.210.180]) by fiordland.canonical.com (Postfix) with ESMTP id 70E3AA18010 for ; Thu, 15 Mar 2012 00:52:12 +0000 (UTC) Received: by iage36 with SMTP id e36so4219765iag.11 for ; Wed, 14 Mar 2012 17:52:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf :mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding:x-gm-message-state; bh=LFtvDPM38o1XDFe1yscY1Xpg7HwYfZ3lw5ByabDoGvs=; b=GBzA2sqCqQ7gYZ7ZdC5s+HKfY+QLFEPqbJsGaze5R1HjoGH+yiFBRGX5DZFY+5pUaO vh961vU9aWPqo24Q6QNB2Je1wLQkXyvQ4Pj+iK0cwgS84SzPMmpy3qmh333ymYV1NHHM 2QVXfWFKA37mgUFicub7CM8pknYLwRkGRKlloCFdveI2lOXhk7xLWacvhYWXgJHM7BXA v/5wIZDlLtlMumpiGaL7Nc89yhJJPB/3xDhcCksv3LOkOtGydUd1ldD0TJP2V4PL4cbo GJ74GlvZSpElISKxZoIerDJbDRRLD0jFujcLnGNmXVVBFu6fdOK178vkAe3pT9iOoA8U 5d8w== Received: by 10.42.145.72 with SMTP id e8mr6017211icv.0.1331772731780; Wed, 14 Mar 2012 17:52:11 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.53.18 with SMTP id k18csp23809ibg; Wed, 14 Mar 2012 17:52:10 -0700 (PDT) Received: by 10.68.223.42 with SMTP id qr10mr257812pbc.127.1331772730247; Wed, 14 Mar 2012 17:52:10 -0700 (PDT) Received: from na3sys009aog124.obsmtp.com ([74.125.149.151]) by mx.google.com with SMTP id l3si766033pbs.244.2012.03.14.17.52.09 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 14 Mar 2012 17:52:10 -0700 (PDT) Received-SPF: pass (google.com: domain of mturquette@ti.com designates 74.125.149.151 as permitted sender) client-ip=74.125.149.151; Authentication-Results: mx.google.com; spf=pass (google.com: domain of mturquette@ti.com designates 74.125.149.151 as permitted sender) smtp.mail=mturquette@ti.com Received: from mail-ey0-f174.google.com ([209.85.215.174]) (using TLSv1) by na3sys009aob124.postini.com ([74.125.148.12]) with SMTP ID DSNKT2E9OK60Wk2mCeLYkwY3o3b/s3qmwRBF@postini.com; Wed, 14 Mar 2012 17:52:09 PDT Received: by mail-ey0-f174.google.com with SMTP id q12so1331331eaa.19 for ; Wed, 14 Mar 2012 17:52:08 -0700 (PDT) Received: by 10.14.40.70 with SMTP id e46mr713991eeb.20.1331772728230; Wed, 14 Mar 2012 17:52:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.213.2.197 with HTTP; Wed, 14 Mar 2012 17:51:48 -0700 (PDT) In-Reply-To: <20120313120507.GS3852@pengutronix.de> References: <1331366064-1273-1-git-send-email-mturquette@linaro.org> <1331366064-1273-3-git-send-email-mturquette@linaro.org> <20120311113414.GM3852@pengutronix.de> <20120312115117.GN3852@pengutronix.de> <20120313120507.GS3852@pengutronix.de> From: "Turquette, Mike" Date: Wed, 14 Mar 2012 17:51:48 -0700 Message-ID: Subject: Re: [PATCH v6 2/3] clk: introduce the common clock framework To: Sascha Hauer Cc: Russell King , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linaro-dev@lists.linaro.org, patches@linaro.org, Jeremy Kerr , Thomas Gleixner , Arnd Bergman , Paul Walmsley , Shawn Guo , Richard Zhao , Saravana Kannan , Magnus Damm , Rob Herring , Mark Brown , Linus Walleij , Stephen Boyd , Amit Kucheria , Deepak Saxena , Grant Likely , Andrew Lunn X-Gm-Message-State: ALoCoQm9hq9wfOkj+CREK8pVuiWc/ToXKxDF1QYzUHChSUfA3PkLHpVFFrKkUIet6Dl1LaUMoqmT On Tue, Mar 13, 2012 at 5:05 AM, Sascha Hauer wrote: > On Mon, Mar 12, 2012 at 08:16:36PM -0700, Turquette, Mike wrote: >> On Mon, Mar 12, 2012 at 4:51 AM, Sascha Hauer wrote: >> > I tried another >> > approach on the weekend which basically does not try to do all in a >> > single recursion but instead sets the rate in multiple steps: >> > >> > 1) call a function which calculates all new rates of affected clocks >> >   in a rate change and safes the value in a clk->new_rate field. This >> >   function returns the topmost clock which has to be changed. >> > 2) starting from the topmost clock notify all clients. This walks the >> >   whole subtree even if a notfifier refuses the change. If necessary >> >   we can walk the whole subtree again to abort the change. >> > 3) actually change rates starting from the topmost clocks and notify >> >   all clients on the way. I changed the set_rate callback to void. >> >   Instead of failing (what is failing in case of set_rate? The clock >> >   will still have some rate) I check for the result with >> >   clk_ops->recalc_rate. > > The way described above works for me now, see this branch: > > git://git.pengutronix.de/git/imx/linux-2.6.git v3.3-rc6-clkv6-fixup > > You may not necessarily like it as it changes quite a lot in the rate > changing code. I tried that code and I really like it! It is much more readable and feels less "fragile" than the previous recursive __clk_set_rate. I did quite a bit of testing with this code today. One of the tests looks like this: pll (adjustable to anything) | clk_divider (5 bits wide) | dummy (no clk_ops) The new code did a fine job arbitrating rates for the PLL and the intermediate divider even if I put weird constraints on the PLL. For instance if I artificially limited it to a minimum of 600MHz and then ran clk_set_rate(dummy, 300MHz) it would lock at 600MHz and set clk_divider to divide-by-2. Setting to 600MHz or more set the divider back to 1 and relocked the PLL appropriately. Pretty cool. I also tested the notifiers with this code and they seem to function properly. I'll take this code in for v7. Thanks a lot for this helpful contribution. I did find that MULT_ROUND_UP caused trouble for my PLL's round_rate implementation. Maybe my PLL code is fragile but a quick fix was to make sure that we send the exact value we want to the round_rate code. I also feel this is more correct. Let me know what you think: 8<--------------------------------------------------------------- commit 189fecedb175d0366759246c4192f45b0bc39a50 Author: Mike Turquette Date: Wed Mar 14 17:29:51 2012 -0700 clk-divider.c: round the actual rate we care about { @@ -84,9 +78,9 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, for (i = 1; i <= maxdiv; i++) { parent_rate = __clk_round_rate(__clk_get_parent(hw->clk), - MULT_ROUND_UP(rate, i)); + (rate * i)); now = parent_rate / i; - if (now <= rate && now >= best) { + if (now <= rate && now > best) { bestdiv = i; best = now; *best_parent_rate = parent_rate; diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index 86ca9cd..06ef4a0 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -47,12 +47,6 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw, } EXPORT_SYMBOL_GPL(clk_divider_recalc_rate); -/* - * The reverse of DIV_ROUND_UP: The maximum number which - * divided by m is r - */ -#define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1) - static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, unsigned long *best_parent_rate)