From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754315Ab2CSL3H (ORCPT ); Mon, 19 Mar 2012 07:29:07 -0400 Received: from metis.ext.pengutronix.de ([92.198.50.35]:33473 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752350Ab2CSL3F (ORCPT ); Mon, 19 Mar 2012 07:29:05 -0400 Date: Mon, 19 Mar 2012 12:28:58 +0100 From: Sascha Hauer To: Rajendra Nayak Cc: Mike Turquette , Russell King , Paul Walmsley , Linus Walleij , patches@linaro.org, Stephen Boyd , Mark Brown , Magnus Damm , linux-kernel@vger.kernel.org, Amit Kucheria , Richard Zhao , Grant Likely , Deepak Saxena , Saravana Kannan , Shawn Guo , linaro-dev@lists.linaro.org, Jeremy Kerr , Arnd Bergman , linux-arm-kernel@lists.infradead.org, Mike Turquette Subject: Re: [PATCH v7 2/3] clk: introduce the common clock framework Message-ID: <20120319112858.GX3852@pengutronix.de> References: <1331878280-2758-1-git-send-email-mturquette@linaro.org> <1331878280-2758-3-git-send-email-mturquette@linaro.org> <4F6716DD.5090900@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F6716DD.5090900@ti.com> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 12:25:13 up 127 days, 19:12, 98 users, load average: 0.00, 0.04, 0.24 User-Agent: Mutt/1.5.21 (2010-09-15) X-SA-Exim-Connect-IP: 2001:6f8:1178:2:21e:67ff:fe11:9c5c X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 19, 2012 at 04:52:05PM +0530, Rajendra Nayak wrote: > Hi Mike, > > >+/* > >+ * calculate the new rates returning the topmost clock that has to be > >+ * changed. > >+ */ > >+static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate) > >+{ > >+ struct clk *top = clk; > >+ unsigned long best_parent_rate = clk->parent->rate; > > Shouldn't you check for a valid parent before dereferencing it? A > clk_set_rate() on a root clock might throw up some issues otherwise. > Yes, should be checked. > >+ unsigned long new_rate; > >+ > >+ if (!clk->ops->round_rate&& !(clk->flags& CLK_SET_RATE_PARENT)) { > >+ clk->new_rate = clk->rate; > >+ return NULL; > > So does this mean a clk_set_rate() fails for a clk which does not have > a valid .round_rate and does not have a CLK_SET_RATE_PARENT flag set? > I was thinking this could do a.. > clk->new_rate = rate; > top = clk; > goto out; > ..instead. The core should make sure that either both set_rate and round_rate are present or none of them. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | From mboxrd@z Thu Jan 1 00:00:00 1970 From: s.hauer@pengutronix.de (Sascha Hauer) Date: Mon, 19 Mar 2012 12:28:58 +0100 Subject: [PATCH v7 2/3] clk: introduce the common clock framework In-Reply-To: <4F6716DD.5090900@ti.com> References: <1331878280-2758-1-git-send-email-mturquette@linaro.org> <1331878280-2758-3-git-send-email-mturquette@linaro.org> <4F6716DD.5090900@ti.com> Message-ID: <20120319112858.GX3852@pengutronix.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Mar 19, 2012 at 04:52:05PM +0530, Rajendra Nayak wrote: > Hi Mike, > > >+/* > >+ * calculate the new rates returning the topmost clock that has to be > >+ * changed. > >+ */ > >+static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate) > >+{ > >+ struct clk *top = clk; > >+ unsigned long best_parent_rate = clk->parent->rate; > > Shouldn't you check for a valid parent before dereferencing it? A > clk_set_rate() on a root clock might throw up some issues otherwise. > Yes, should be checked. > >+ unsigned long new_rate; > >+ > >+ if (!clk->ops->round_rate&& !(clk->flags& CLK_SET_RATE_PARENT)) { > >+ clk->new_rate = clk->rate; > >+ return NULL; > > So does this mean a clk_set_rate() fails for a clk which does not have > a valid .round_rate and does not have a CLK_SET_RATE_PARENT flag set? > I was thinking this could do a.. > clk->new_rate = rate; > top = clk; > goto out; > ..instead. The core should make sure that either both set_rate and round_rate are present or none of them. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |