Re: [PATCH v7 2/3] clk: introduce the common clock framework

[Saravana Kannan]

On 03/19/2012 12:33 PM, Turquette, Mike wrote:
> On Mon, Mar 19, 2012 at 12:13 PM, Saravana Kannan
> <skannan@xxxxxxxxxxxxxx>  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:
>
> 1) 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;

> 2) 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
--
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/