From patchwork Wed Feb 6 06:10:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hiroshi DOYU X-Patchwork-Id: 2102081 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 7C5A43FCFC for ; Wed, 6 Feb 2013 06:13:26 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1U2yDn-0002cJ-Db; Wed, 06 Feb 2013 06:11:03 +0000 Received: from hqemgate03.nvidia.com ([216.228.121.140]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1U2yDf-0002bM-2R for linux-arm-kernel@lists.infradead.org; Wed, 06 Feb 2013 06:10:59 +0000 Received: from hqnvupgp08.nvidia.com (Not Verified[216.228.121.13]) by hqemgate03.nvidia.com id ; Tue, 05 Feb 2013 22:15:25 -0800 Received: from hqemhub01.nvidia.com ([172.17.108.22]) by hqnvupgp08.nvidia.com (PGP Universal service); Tue, 05 Feb 2013 22:07:51 -0800 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Tue, 05 Feb 2013 22:07:51 -0800 Received: from deemhub02.nvidia.com (10.21.69.138) by hqemhub01.nvidia.com (172.20.150.30) with Microsoft SMTP Server (TLS) id 8.3.297.1; Tue, 5 Feb 2013 22:10:53 -0800 Received: from DEMAIL01.nvidia.com ([10.21.69.139]) by deemhub02.nvidia.com ([10.21.69.138]) with mapi; Wed, 6 Feb 2013 07:10:50 +0100 From: Hiroshi Doyu To: Prashant Gaikwad Date: Wed, 6 Feb 2013 07:10:48 +0100 Subject: Re: [PATCH V2] clk: Add composite clock type Thread-Topic: [PATCH V2] clk: Add composite clock type Thread-Index: Ac4EMLXjOqp4UCOiSWGgf08YSnBDww== Message-ID: <20130206.081048.71241785637713947.hdoyu@nvidia.com> References: <5110C3E5.2010503@nvidia.com><20130205.122252.570646990867457667.hdoyu@nvidia.com><5111C604.8070104@nvidia.com> In-Reply-To: <5111C604.8070104@nvidia.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-nvconfidentiality: public acceptlanguage: en-US MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130206_011055_237743_9111AE11 X-CRM114-Status: GOOD ( 17.57 ) X-Spam-Score: -7.6 (-------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-7.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [216.228.121.140 listed in list.dnswl.org] -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: "mturquette@linaro.org" , "swarren@wwwdotorg.org" , "t.figa@samsung.com" , "sboyd@codeaurora.org" , "linux-kernel@vger.kernel.org" , "linux-tegra@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Prashant Gaikwad wrote @ Wed, 6 Feb 2013 03:55:00 +0100: > >> No, clk_ops depends on the clocks you are using. There could be a clock > >> with mux and gate while another one with mux and div. > > You are right. What about the following? We don't have to have similar > > copy of clk_composite_ops for each instances. > > Clock framework takes decision depending on the ops availability and it > does not know if the clock is mux or gate. > > For example, > > if (clk->ops->enable) { > ret = clk->ops->enable(clk->hw); > if (ret) { > __clk_disable(clk->parent); > return ret; > } > } > > in above case if clk_composite does not have gate clock then as per your > suggestion if it returns error value then it will fail and it is wrong. Ok, now I understand. Thank you for explanation. We always need to allocate clk_composite_ops for each clk_composite, right? If so what about having "struct clk_ops ops" in "struct clk_composite"? > > diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c > > index f30fb4b..8f88805 100644 > > --- a/drivers/clk/clk-composite.c > > +++ b/drivers/clk/clk-composite.c > > @@ -27,6 +27,9 @@ static u8 clk_composite_get_parent(struct clk_hw *hw) > > const struct clk_ops *mux_ops = composite->mux_ops; > > struct clk_hw *mux_hw = composite->mux_hw; > > > > + if (!mux_hw->clk) > > + return -EINVAL; > > + > > mux_hw->clk = hw->clk; > > It is wrong. Will the above "mux_hw->clk = hw->clk" be removed from the original? diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index f30fb4b..5240e24 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -129,20 +129,13 @@ struct clk *clk_register_composite(struct device *dev, const char *name, pr_err("%s: could not allocate composite clk\n", __func__); return ERR_PTR(-ENOMEM); } + clk_composite_ops = &composite->ops; init.name = name; init.flags = flags | CLK_IS_BASIC; init.parent_names = parent_names; init.num_parents = num_parents; - /* allocate the clock ops */ - clk_composite_ops = kzalloc(sizeof(*clk_composite_ops), GFP_KERNEL); - if (!clk_composite_ops) { - pr_err("%s: could not allocate clk ops\n", __func__); - kfree(composite); - return ERR_PTR(-ENOMEM); - } - if (mux_hw && mux_ops) { if (!mux_ops->get_parent || !mux_ops->set_parent) { clk = ERR_PTR(-EINVAL); @@ -202,7 +195,6 @@ struct clk *clk_register_composite(struct device *dev, const char *name, return clk; err: - kfree(clk_composite_ops); kfree(composite); return clk; } diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index f0ac818..bb5d36a 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -346,6 +346,8 @@ struct clk_composite { const struct clk_ops *mux_ops; const struct clk_ops *div_ops; const struct clk_ops *gate_ops; + + const struct clk_ops ops; }; struct clk *clk_register_composite(struct device *dev, const char *name,