Re: [PATCH] clk: Constify struct clk_init_data

Re: [PATCH] clk: Constify struct clk_init_data

[Saravana Kannan]

On 05/14/2012 02:53 PM, Turquette, Mike wrote:
> On Mon, May 14, 2012 at 7:12 AM, Mark Brown<broonie@opensource.wolfsonmicro.com>  wrote:
>> Allow drivers to declare their clk_init_data const, the framework really
>> shouldn't be modifying the data.
>>
>> Signed-off-by: Mark Brown<broonie@opensource.wolfsonmicro.com>
>
> +interested parties
>
> Mark, I like this change but it's reminded me of a few things I meant to
> bring up on the list in the past.  Certainly some folks will mark their
> struct clk_hw_init data as __initconst.  Currently none of the
> documentation mentions that fact and I'm a bit worried about clk code
> which assumes that hw->init will always be around and freely accesses
> it.
>
> I think that, as a rule, hw->init cannot be assumed to be valid after
> clk_register returns.  Would anyone else like to weigh in on it?  If so
> then I'll cook up a follow-up patch to reflect this in the kerneldoc.

If you mean hw->init pointer can't be assumed to be point to anything 
valid after clk_register or __clk_register, then yes, I agree with that. 
But the actual data that hw->init pointed to might still be around and 
referenced by clk if __clk_register is used.

Btw, I didn't follow up on the other thread we were having, but can you 
remind me again what was the reason that you thought that only 
__clk_init() would work for your static init code and __clk_register() 
won't work? Or did I just misunderstand your comment that was trying to 
say that you didn't want to switch to __clk_register close to kernel 
release?

There are some older threads where I think we are a bit out of sync on 
the init data (I thought I did what you said you preferred, but you 
asked me why I did that). I will try to restart them.

Thanks,
Saravana

>
> Thanks,
> Mike
>
>> ---
>>   include/linux/clk-provider.h |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index c1c23b9..fc43ea6 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -143,7 +143,7 @@ struct clk_init_data {
>>   */
>>   struct clk_hw {
>>         struct clk *clk;
>> -       struct clk_init_data *init;
>> +       const struct clk_init_data *init;
>>   };
>>
>>   /*
>> --
>> 1.7.10
>>
>


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] clk: Constify struct clk_init_data

[Rajendra Nayak]

Hi Saravana,

On Tuesday 15 May 2012 06:38 AM, Saravana Kannan wrote:
> Btw, I didn't follow up on the other thread we were having, but can you
> remind me again what was the reason that you thought that only
> __clk_init() would work for your static init code and __clk_register()
> won't work?

One of the main reason has been the platform implementation we have to
handle some complex mux/divider combo clocks in OMAP2/3 which rely on
'struct clk' pointers. Maybe we can do away with the existing
implementation and redo it so we don't have any such limitation, but the
quantum of change moving to common clk has been so much that we are
trying to minimize on the platform code changes for now. So while
we move to common clk it would still be useful to have __clk_init()
around for a while till we figure out how to get rid of it for OMAP.

regards,
Rajendra

Re: [PATCH] clk: Constify struct clk_init_data

[Turquette, Mike]

On Mon, May 14, 2012 at 10:13 PM, Rajendra Nayak <rnayak@ti.com> wrote:
> Hi Saravana,
>
>
> On Tuesday 15 May 2012 06:38 AM, Saravana Kannan wrote:
>>
>> Btw, I didn't follow up on the other thread we were having, but can you
>> remind me again what was the reason that you thought that only
>> __clk_init() would work for your static init code and __clk_register()
>> won't work?
>
>
> One of the main reason has been the platform implementation we have to
> handle some complex mux/divider combo clocks in OMAP2/3 which rely on
> 'struct clk' pointers. Maybe we can do away with the existing
> implementation and redo it so we don't have any such limitation, but the
> quantum of change moving to common clk has been so much that we are
> trying to minimize on the platform code changes for now. So while
> we move to common clk it would still be useful to have __clk_init()
> around for a while till we figure out how to get rid of it for OMAP.
>

Just to add to what Rajendra has stated for OMAP: after OMAP's
conversion is finally made initcall-able then I'll get rid of lots of
the gross macros and before-the-memory-allocator-is-alive stuff.  I'll
make sure that no other platforms are using those bits of course, but
I plan on removing some of this stuff from the core once my platform
is up to speed.

Regards,
Mike

> regards,
> Rajendra