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

[Turquette, Mike]

On Thu, Apr 5, 2012 at 6:30 PM, Saravana Kannan <skannan@xxxxxxxxxxxxxx> wrote:
> On 03/27/2012 11:49 AM, Turquette, Mike wrote:
>>
>> On Mon, Mar 26, 2012 at 9:35 PM, Saravana Kannan<skannan@xxxxxxxxxxxxxx>
>>  wrote:
>>>
>>> Mike,
>>>
>>> (*nudge*) (*nudge*)
>>
>>
>> Hi Saravana,
>>
>> I spent some time mulling over the ideas this weekend and I agree that
>> we have 3 types of data, so having the three structs makes some sense.
>>  I'm checking this patch out more earnestly today.
>
>
> (*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.

Sorry for the long wait Saravana.  I've been swamped the past few days.

I thought about splitting this interface further (basically making the
statically initialized stuff visible to code that includes
clk-provider.h) and it seems like the benefits outweight the negative
stuff.

However I want to advance the cause of __initdata clock initializers,
so I think the final version of this change should have the core
perform a copy of the initializer data into struct clk_hw.  It's been
a while since I looked at Sascha's clk_initalizer patches but I think
that it is essentially the same idea.

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.

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?

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?

Regards,
Mike
--
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/