Re: [PATCH 2/2] clk: Move init fields from clk to clk_hw

From: Saravana Kannan
Date: Wed Apr 11 2012 - 15:57:53 EST


On 04/11/2012 10:59 AM, Turquette, Mike wrote:
On Thu, Apr 5, 2012 at 6:30 PM, Saravana Kannan<skannan@xxxxxxxxxxxxxx> wrote:
(*nudge*) (*nudge*) again :-)

Sorry to keep nudging you on this often. The longer we wait for this, the
more churn it will cause later on. That's why I'm being persistent on this.
Also, once this goes in, I can start working on upstreaming MSM clock
support.

<SNIP>

TL;DR: I think a combination of your change to expose the
not-so-private parts of struct clk into struct clk_hw are OK and have
real benefits for the clk-provider.h code. However we need to
implement it such that all the statically initialized data can be
__initdata.

Sounds like a reasonable compromise.

My only concern with this series is that platform clock code will
access struct clk_hw members without the appropriate lock held (namely
prepare_lock). I'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...).
Besides clk_put are there any real instances where this might happen?

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 their data as __init_data and access them later, then there is not much we can do. Also, I believe the compiler throws out some warning when you try to access __init_data from non-init code.

I'll be pushing my fixes branch out to the list soon. Do 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 my patch. But I can try to rebase mine on top of your changes. Hopefully your fixes aren't crazy big/complex.

This is a busy week for me at work. I will try to send a patch in a day or two.

Thanks,
Saravana

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/