From patchwork Sun Apr 19 12:13:04 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 6238701 Return-Path: X-Original-To: patchwork-linux-rockchip@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id EC580BF4A6 for ; Sun, 19 Apr 2015 17:06:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 11F232034E for ; Sun, 19 Apr 2015 17:06:05 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6CDAA2034C for ; Sun, 19 Apr 2015 17:06:02 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YjsfQ-0007Ls-Sw; Sun, 19 Apr 2015 17:06:00 +0000 Received: from gloria.sntech.de ([95.129.55.99]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Yjo6t-0000wV-Nl; Sun, 19 Apr 2015 12:14:04 +0000 Received: from ip92344031.dynamic.kabel-deutschland.de ([146.52.64.49] helo=diego.localnet) by gloria.sntech.de with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1Yjo5x-0004OA-H0; Sun, 19 Apr 2015 14:13:05 +0200 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Boris Brezillon Subject: Re: [PATCH 1/2] clk: change clk_ops' ->round_rate() prototype Date: Sun, 19 Apr 2015 14:13:04 +0200 Message-ID: <7408975.lBcgZIN9hf@diego> User-Agent: KMail/4.14.1 (Linux/3.16.0-4-amd64; KDE/4.14.2; x86_64; ; ) In-Reply-To: <1429255769-13639-2-git-send-email-boris.brezillon@free-electrons.com> References: <1429255769-13639-1-git-send-email-boris.brezillon@free-electrons.com> <1429255769-13639-2-git-send-email-boris.brezillon@free-electrons.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150419_051403_960694_A2A73BCE X-CRM114-Status: GOOD ( 16.94 ) X-Spam-Score: -0.0 (/) X-Mailman-Approved-At: Sun, 19 Apr 2015 10:05:59 -0700 Cc: linux-mips@linux-mips.org, Ulf Hansson , Prashant Gaikwad , linux-doc@vger.kernel.org, Tony Lindgren , Liviu Dudau , Tomasz Figa , dri-devel@lists.freedesktop.org, Max Filippov , Thierry Reding , Sylwester Nawrocki , Daniel Walker , Alexandre Courbot , Lorenzo Pieralisi , Mike Turquette , Jonathan Corbet , Emilio =?ISO-8859-1?Q?L=F3pez?= , patches@opensource.wolfsonmicro.com, Michal Simek , linux-rockchip@lists.infradead.org, David Brown , linux-media@vger.kernel.org, Philipp Zabel , rtc-linux@googlegroups.com, Stephen Warren , linux-arm-msm@vger.kernel.org, spear-devel@list.st.com, Barry Song , Mikko Perttunen , linux-samsung-soc@vger.kernel.org, linux-tegra@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Paul Walmsley , Peter De Schrijver , linux-kernel@vger.kernel.org, Ralf Baechle , Tero Kristo , Bryan Huntsman , Viresh Kumar , ascha Hauer , Sudeep Holla , Maxime Ripard , Shawn Guo X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Boris, Am Freitag, 17. April 2015, 09:29:28 schrieb Boris Brezillon: > Clock rates are stored in an unsigned long field, but ->round_rate() > (which returns a rounded rate from a requested one) returns a long > value (errors are reported using negative error codes), which can lead > to long overflow if the clock rate exceed 2Ghz. > > Change ->round_rate() prototype to return 0 or an error code, and pass the > requested rate as a pointer so that it can be adjusted depending on > hardware capabilities. > > Signed-off-by: Boris Brezillon > --- On a rk3288-veyron-pinky with the fix described below: Tested-by: Heiko Stuebner > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index fa5a00e..1462ddc 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1640,8 +1643,10 @@ static struct clk_core *clk_calc_new_rates(struct > clk_core *clk, &parent_hw); > parent = parent_hw ? parent_hw->core : NULL; > } else if (clk->ops->round_rate) { > - new_rate = clk->ops->round_rate(clk->hw, rate, > - &best_parent_rate); > + if (clk->ops->round_rate(clk->hw, &new_rate, > + &best_parent_rate)) > + return NULL; > + > if (new_rate < min_rate || new_rate > max_rate) > return NULL; > } else if (!parent || !(clk->flags & CLK_SET_RATE_PARENT)) { This is using new_rate uninitialized when calling into the round_rate callback. Which in turn pushed my PLLs up to 2.2GHz :-) I guess you'll need something like the following: > diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c > index f8d3baf..bd408ef 100644 > --- a/drivers/clk/rockchip/clk-pll.c > +++ b/drivers/clk/rockchip/clk-pll.c > @@ -63,8 +63,8 @@ static const struct rockchip_pll_rate_table > *rockchip_get_pll_settings( return NULL; > } > > -static long rockchip_pll_round_rate(struct clk_hw *hw, > - unsigned long drate, unsigned long *prate) > +static int rockchip_pll_round_rate(struct clk_hw *hw, > + unsigned long *drate, unsigned long *prate) > { > struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > const struct rockchip_pll_rate_table *rate_table = pll->rate_table; > @@ -72,12 +72,15 @@ static long rockchip_pll_round_rate(struct clk_hw *hw, > > /* Assumming rate_table is in descending order */ > for (i = 0; i < pll->rate_count; i++) { > - if (drate >= rate_table[i].rate) > - return rate_table[i].rate; > + if (*drate >= rate_table[i].rate) { > + *drate = rate_table[i].rate; > + return 0; > + } > } > > /* return minimum supported value */ > - return rate_table[i - 1].rate; > + *drate = rate_table[i - 1].rate; > + return 0; > } > > /* The rockchip-part: Reviewed-by: Heiko Stuebner And as I've stumbled onto this recently too, the clock-maintainership has expanded to Stephen Boyd and linux-clk@vger.kernel.org . Heiko diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index db4e4b2..afc7733 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1605,6 +1605,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *clk, &parent_hw); parent = parent_hw ? parent_hw->core : NULL; } else if (clk->ops->round_rate) { + new_rate = rate; if (clk->ops->round_rate(clk->hw, &new_rate, &best_parent_rate)) return NULL;