From patchwork Tue Feb 5 10:22:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hiroshi DOYU X-Patchwork-Id: 2097221 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id A26A2DF24C for ; Tue, 5 Feb 2013 10:25:49 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1U2fg7-0002Su-W0; Tue, 05 Feb 2013 10:23:03 +0000 Received: from hqemgate04.nvidia.com ([216.228.121.35]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1U2fg5-0002SF-ID for linux-arm-kernel@lists.infradead.org; Tue, 05 Feb 2013 10:23:02 +0000 Received: from hqnvupgp05.nvidia.com (Not Verified[216.228.121.13]) by hqemgate04.nvidia.com id ; Tue, 05 Feb 2013 02:22:47 -0800 Received: from hqemhub03.nvidia.com ([172.17.108.22]) by hqnvupgp05.nvidia.com (PGP Universal service); Tue, 05 Feb 2013 02:22:58 -0800 X-PGP-Universal: processed; by hqnvupgp05.nvidia.com on Tue, 05 Feb 2013 02:22:58 -0800 Received: from deemhub01.nvidia.com (10.21.69.137) by hqemhub03.nvidia.com (172.20.150.15) with Microsoft SMTP Server (TLS) id 8.3.297.1; Tue, 5 Feb 2013 02:22:58 -0800 Received: from DEMAIL01.nvidia.com ([10.21.69.139]) by deemhub01.nvidia.com ([10.21.69.137]) with mapi; Tue, 5 Feb 2013 11:22:54 +0100 From: Hiroshi Doyu To: Prashant Gaikwad Date: Tue, 5 Feb 2013 11:22:52 +0100 Subject: Re: [PATCH V2] clk: Add composite clock type Thread-Topic: [PATCH V2] clk: Add composite clock type Thread-Index: Ac4DisJ0TR+q27gqTrybRcC548B9Lg== Message-ID: <20130205.122252.570646990867457667.hdoyu@nvidia.com> References: <1359965482-29655-1-git-send-email-pgaikwad@nvidia.com><20130204.113739.2227266298512077917.hdoyu@nvidia.com><5110C3E5.2010503@nvidia.com> In-Reply-To: <5110C3E5.2010503@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-20130205_052301_733135_C3A50315 X-CRM114-Status: GOOD ( 14.96 ) 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.35 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" , "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 @ Tue, 5 Feb 2013 09:33:41 +0100: > > The members of "clk_composite_ops" seems to be always assigned > > statically. Istead of dynamically allocating/assigning, can't we just > > have "clk_composite_ops" statically as below? > > > > static struct clk_ops clk_composite_ops = { > > .get_parent = clk_composite_get_parent; > > .set_parent = clk_composite_set_parent; > > .recalc_rate = clk_composite_recalc_rate; > > .round_rate = clk_composite_round_rate; > > .set_rate = clk_composite_set_rate; > > .is_enabled = clk_composite_is_enabled; > > .enable = clk_composite_enable; > > .disable = clk_composite_disable; > > }; > > > > struct clk *clk_register_composite(struct device *dev, const char *name, > > const char **parent_names, int num_parents, > > struct clk_hw *mux_hw, const struct clk_ops *mux_ops, > > struct clk_hw *div_hw, const struct clk_ops *div_ops, > > struct clk_hw *gate_hw, const struct clk_ops *gate_ops, > > unsigned long flags) > > { > > ..... > > > > init.ops = &clk_composite_ops; > > 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. 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; return mux_ops->get_parent(mux_hw); @@ -38,6 +41,9 @@ static int clk_composite_set_parent(struct clk_hw *hw, u8 index) 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; return mux_ops->set_parent(mux_hw, index); @@ -50,6 +56,9 @@ static unsigned long clk_composite_recalc_rate(struct clk_hw *hw, const struct clk_ops *div_ops = composite->div_ops; struct clk_hw *div_hw = composite->div_hw; + if (!div_hw->clk) + return -EINVAL; + div_hw->clk = hw->clk; return div_ops->recalc_rate(div_hw, parent_rate); @@ -62,6 +71,9 @@ static long clk_composite_round_rate(struct clk_hw *hw, unsigned long rate, const struct clk_ops *div_ops = composite->div_ops; struct clk_hw *div_hw = composite->div_hw; + if (!div_hw->clk) + return -EINVAL; + div_hw->clk = hw->clk; return div_ops->round_rate(div_hw, rate, prate); @@ -74,6 +86,9 @@ static int clk_composite_set_rate(struct clk_hw *hw, unsigned long rate, const struct clk_ops *div_ops = composite->div_ops; struct clk_hw *div_hw = composite->div_hw; + if (!div_hw->clk) + return -EINVAL; + div_hw->clk = hw->clk; return div_ops->set_rate(div_hw, rate, parent_rate); @@ -85,6 +100,9 @@ static int clk_composite_is_enabled(struct clk_hw *hw) const struct clk_ops *gate_ops = composite->gate_ops; struct clk_hw *gate_hw = composite->gate_hw; + if (!gate_hw->clk) + return -EINVAL; + gate_hw->clk = hw->clk; return gate_ops->is_enabled(gate_hw); @@ -96,6 +114,9 @@ static int clk_composite_enable(struct clk_hw *hw) const struct clk_ops *gate_ops = composite->gate_ops; struct clk_hw *gate_hw = composite->gate_hw; + if (!gate_hw->clk) + return -EINVAL; + gate_hw->clk = hw->clk; return gate_ops->enable(gate_hw); @@ -107,11 +128,25 @@ static void clk_composite_disable(struct clk_hw *hw) const struct clk_ops *gate_ops = composite->gate_ops; struct clk_hw *gate_hw = composite->gate_hw; + if (!gate_hw->clk) + return -EINVAL; + gate_hw->clk = hw->clk; gate_ops->disable(gate_hw); } +static struct clk_ops clk_composite_ops = { + .get_parent = clk_composite_get_parent, + .set_parent = clk_composite_set_parent, + .recalc_rate = clk_composite_recalc_rate, + .round_rate = clk_composite_round_rate, + .set_rate = clk_composite_set_rate, + .is_enabled = clk_composite_is_enabled, + .enable = clk_composite_enable, + .disable = clk_composite_disable, +}; + struct clk *clk_register_composite(struct device *dev, const char *name, const char **parent_names, int num_parents, struct clk_hw *mux_hw, const struct clk_ops *mux_ops, @@ -135,14 +170,6 @@ struct clk *clk_register_composite(struct device *dev, const char *name, 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); @@ -151,8 +178,6 @@ struct clk *clk_register_composite(struct device *dev, const char *name, composite->mux_hw = mux_hw; composite->mux_ops = mux_ops; - clk_composite_ops->get_parent = clk_composite_get_parent; - clk_composite_ops->set_parent = clk_composite_set_parent; } if (div_hw && div_ops) { @@ -164,9 +189,6 @@ struct clk *clk_register_composite(struct device *dev, const char *name, composite->div_hw = div_hw; composite->div_ops = div_ops; - clk_composite_ops->recalc_rate = clk_composite_recalc_rate; - clk_composite_ops->round_rate = clk_composite_round_rate; - clk_composite_ops->set_rate = clk_composite_set_rate; } if (gate_hw && gate_ops) { @@ -178,12 +200,9 @@ struct clk *clk_register_composite(struct device *dev, const char *name, composite->gate_hw = gate_hw; composite->gate_ops = gate_ops; - clk_composite_ops->is_enabled = clk_composite_is_enabled; - clk_composite_ops->enable = clk_composite_enable; - clk_composite_ops->disable = clk_composite_disable; } - init.ops = clk_composite_ops; + init.ops = &clk_composite_ops; composite->hw.init = &init; clk = clk_register(dev, &composite->hw); @@ -202,7 +221,6 @@ struct clk *clk_register_composite(struct device *dev, const char *name, return clk; err: - kfree(clk_composite_ops); kfree(composite); return clk; }