From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Turquette, Mike" Subject: Re: [PATCH 2/2] clk: Move init fields from clk to clk_hw Date: Wed, 11 Apr 2012 13:17:20 -0700 Message-ID: References: <1332214706-675-1-git-send-email-skannan@codeaurora.org> <1332214706-675-2-git-send-email-skannan@codeaurora.org> <20120320072018.GC32469@S2101-09.ap.freescale.net> <20120320094031.GI3852@pengutronix.de> <20120320141811.GF32469@S2101-09.ap.freescale.net> <20120320181050.GN3852@pengutronix.de> <4F68E34A.70207@codeaurora.org> <20120320231242.GP3852@pengutronix.de> <4F694484.1030706@codeaurora.org> <4F714397.6080608@codeaurora.org> <4F7E4752.4020007@codeaurora.org> <4F85E238.9050102@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <4F85E238.9050102@codeaurora.org> Sender: linux-kernel-owner@vger.kernel.org To: Saravana Kannan Cc: Sascha Hauer , Andrew Lunn , Grant Likely , Jamie Iles , Jeremy Kerr , Magnus Damm , Deepak Saxena , linux-arm-kernel@lists.infradead.org, Arnd Bergman , linux-arm-msm@vger.kernel.org, Rob Herring , Russell King , Thomas Gleixner , Richard Zhao , Shawn Guo , Paul Walmsley , Linus Walleij , Mark Brown , Stephen Boyd , linux-kernel@vger.kernel.org, Amit Kucheria , Shawn Guo List-Id: linux-arm-msm@vger.kernel.org On Wed, Apr 11, 2012 at 12:57 PM, Saravana Kannan wrote: > On 04/11/2012 10:59 AM, Turquette, Mike wrote: >> On Thu, Apr 5, 2012 at 6:30 PM, Saravana Kannan >> My only concern with this series is that platform clock code will >> access struct clk_hw members without the appropriate lock held (name= ly >> prepare_lock). =A0I'm a bit worried that there might be a case where >> some clock code access clk_hw->name when clk_hw has somehow been >> destroyed/altered (e.g. if clk_put every finally gets implemented...= ). >> =A0Besides clk_put are there any real instances where this might hap= pen? > > This problem is true for any struct/field marked as __init_data. So, = I don't > think this is a real problem. If someone is stupid enough to mark the= ir data > as __init_data and access them later, then there is not much we can d= o. > Also, I believe the compiler throws out some warning when you try to = access > __init_data from non-init code. I'm referring to platform clock code accessing clk_hw->name or clk_hw->parent_names AFTER the core code has copied the initialization data. So this isn't an __initdata problem as much as it is a design issue with exposing some members of struct clk_hw. I think that this is basically OK since the benefits of not having to use so many helpers functions in platform clock code is obvious and if a platform gets bitten by this then they will learn the hard way to honor the locking semantics outlined in Documentation/clk.txt >> I'll be pushing my fixes branch out to the list soon. =A0Do you want= to >> rebase this change on top of it taking into account the __initdata >> bits? > > I thought it might be easier for you to base your changes on top of m= y > patch. But I can try to rebase mine on top of your changes. Hopefully= your > fixes aren't crazy big/complex. I don't think the fixes are a big deal, especially in the core code. > This is a busy week for me at work. I will try to send a patch in a d= ay or > two. OK. I might take a swing at it myself if I have time. Regards, Mike