[PATCH 01/10] Add a common struct clk

[Linus Walleij]

2011/4/23 Thomas Gleixner <tglx at linutronix.de>:
> On Sat, 23 Apr 2011, Richard Zhao wrote:
>> On Fri, Apr 22, 2011 at 11:23:49AM +0200, Thomas Gleixner wrote:
>> > On Fri, 22 Apr 2011, Richard Zhao wrote:
>> > > > One does a try_module_get(clk->owner), which should be done in generic
>> > > > code. The other does special clock enabling magic which wants to go to
>> > > > clk->prepare().
>> > > >
>> > > You forget mach-u300 __clk_get. It updates some registers.
>> > > The ops get/put let arch clock driver have a chance to do special things at the
>> > > time when a user begins to use the clock.
>> >
>> > No I did not forget. It's exactly the code which wants to go into
>> > clk->prepare. Which you have to call _before_ doing anything with the
>> > clock, so there is no need for an extra callback.
>>
>> No. clk_prepare only have to be called before clk_enable, but not have to for
>> other operations, set_rate/set_parent etc.
>
> And why do you need those registers set before you actually
> prepare/enable the clock? Just to set the rate register? You can store
> that information and set it on prepare. Before prepare/enable the
> value of the rate register is totally irrelevant.
>
> The point of anything whats named get and put in the kernel is to
> obtain or drop a reference to an object.
>
> Putting arbitrary initialization into such functions is a really bad
> hack.
>
> And looking at that u300 code, it's borken:
>
>    user1: clk_get("MCLK");
>           clk_set_rate(.....);
>
>    user2: clk_get("MCLK");
>
> The second clk_get() call will overwrite what ?

Heh yeah that is the case, and this code can just as well live in the
->prepare() function.

The clock that goes to "MCLK" and "MSPRO" is utilizing a common
clock divider, which sounds backwards, but in practice this is because
it's and MMC or Memory Stick Pro block clock, and you cannot pinmux
in both in the ASIC, they are connected to the same pins and either
function need its own support electronics. So naturally only one of
them is ever active at the same time.

This should be *properly* reflected by creating the pin/padmux
framework and have the clock prepare() call hook back into that,
so that when you try to activate say "mclk" you need to mux in these
pins and if the mspro is muxed in at that time you return -ENODEV
and the clock code bails out.

We should just move it to prepare() and I'll fix it up if need be.

Linus Walleij