On 03/19/2012 12:33 PM, Turquette, Mike wrote:
On Mon, Mar 19, 2012 at 12:13 PM, Saravana Kannan skannan@codeaurora.org wrote:
I saw some responses from you over the weekend but not to mine. So, I assumed you were busy with other stuff and I started working on a patch on top of v7.
I only answer trivial emails on the weekend ;-)
I will send that out if I get around to finishing it before you do. Hope that's alright with you.
I'm happy to for you to take a crack at it. I don't know what your implementation looks like, but here are a couple concerns I have:
- if you're copying the data from the initializer over to the struct
clk then make sure you handle the __init data aspects of it properly
Not copying them into clk again. Just leaving them in clk_hw and using them as is. The only fields moved into clk_hw are: + const char *name; + const struct clk_ops *ops; + char **parent_names; + u8 num_parents; + unsigned long flags;
- are the members of struct clk_hw visible to the rest of the world?
Are they modifiable (i.e., not const)? This is undesirable.
I'm leaving the const as is. Some of them you can't mark as const if you want to dynamically create them.
Yes, they are visible to all the platform drivers (not the rest of the world). But these values are provided by the platform drivers anyway, so I don't see a problem with it.
I also don't see any useful hacks a platform driver can do by messing with this fields without crashing the kernel since they don't have access to the locks.
flags might be the one that provides some possibilities since I think you look at them often in the core code. We could just copy it into clk if people really feel strongly about it. At the worst case, we can have a full copy of all these fields inside clk and copy all of them over, but I think that would be overkill for things like names, ops and parent info.
Thanks, Saravana