From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH] clk: Use a separate struct for holding init data. Date: Wed, 2 May 2012 11:02:25 +0100 Message-ID: <20120502100225.GB24211@n2100.arm.linux.org.uk> References: <1335419936-10881-1-git-send-email-skannan@codeaurora.org> <20120502095816.GG20478@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from caramon.arm.linux.org.uk ([78.32.30.218]:33528 "EHLO caramon.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751448Ab2EBKDA (ORCPT ); Wed, 2 May 2012 06:03:00 -0400 Content-Disposition: inline In-Reply-To: <20120502095816.GG20478@pengutronix.de> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Sascha Hauer Cc: Saravana Kannan , Mike Turquette , Arnd Bergman , linux-arm-kernel@lists.infradead.org, Andrew Lunn , Paul Walmsley , Linus Walleij , Stephen Boyd , linux-arm-msm@vger.kernel.org, Mark Brown , Magnus Damm , linux-kernel@vger.kernel.org, Rob Herring , Richard Zhao , Grant Likely , Deepak Saxena , Amit Kucheria , Jamie Iles , Jeremy Kerr , Thomas Gleixner , Shawn Guo On Wed, May 02, 2012 at 11:58:16AM +0200, Sascha Hauer wrote: > > diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c > > index 6e58f11..8e97491 100644 > > --- a/drivers/clk/clk-mux.c > > +++ b/drivers/clk/clk-mux.c > > @@ -95,6 +95,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > > { > > struct clk_mux *mux; > > struct clk *clk; > > + struct clk_init_data init; > > > > /* allocate the mux */ > > mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL); > > @@ -103,6 +104,12 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > > return ERR_PTR(-ENOMEM); > > } > > > > + init.name = name; > > + init.ops = &clk_mux_ops; > > + init.flags = flags; > > + init.parent_names = parent_names; > > + init.num_parents = num_parents; > > + > > /* struct clk_mux assignments */ > > mux->reg = reg; > > mux->shift = shift; > > @@ -110,8 +117,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > > mux->flags = clk_mux_flags; > > mux->lock = lock; > > There is a mux->hw.init = &init missing here. What happens to mux->hw.init long term? Because once this function returns, that pointer will no longer be valid. It would be a good idea to NULL it out in clk_register() once its done with, to ensure that no one comes up with the idea of using it later. From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@arm.linux.org.uk (Russell King - ARM Linux) Date: Wed, 2 May 2012 11:02:25 +0100 Subject: [PATCH] clk: Use a separate struct for holding init data. In-Reply-To: <20120502095816.GG20478@pengutronix.de> References: <1335419936-10881-1-git-send-email-skannan@codeaurora.org> <20120502095816.GG20478@pengutronix.de> Message-ID: <20120502100225.GB24211@n2100.arm.linux.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, May 02, 2012 at 11:58:16AM +0200, Sascha Hauer wrote: > > diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c > > index 6e58f11..8e97491 100644 > > --- a/drivers/clk/clk-mux.c > > +++ b/drivers/clk/clk-mux.c > > @@ -95,6 +95,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > > { > > struct clk_mux *mux; > > struct clk *clk; > > + struct clk_init_data init; > > > > /* allocate the mux */ > > mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL); > > @@ -103,6 +104,12 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > > return ERR_PTR(-ENOMEM); > > } > > > > + init.name = name; > > + init.ops = &clk_mux_ops; > > + init.flags = flags; > > + init.parent_names = parent_names; > > + init.num_parents = num_parents; > > + > > /* struct clk_mux assignments */ > > mux->reg = reg; > > mux->shift = shift; > > @@ -110,8 +117,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > > mux->flags = clk_mux_flags; > > mux->lock = lock; > > There is a mux->hw.init = &init missing here. What happens to mux->hw.init long term? Because once this function returns, that pointer will no longer be valid. It would be a good idea to NULL it out in clk_register() once its done with, to ensure that no one comes up with the idea of using it later.