Re: moving Tegra30 to the common clock framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 7, 2012 at 10:07 PM, zhoujie wu <zhoujiewu@xxxxxxxxx> wrote:
> Hi Mike,
> Could you please explain more details about how to implement a
> re-parenting operation as part of it's .set_rate implementation?

Sure.

> As far as I know, we can not call clk_set_parent in .set_rate function
> directly, since clk_set_rate and clk_set_parent are using the same
> prepare_lock.

That is correct.

> Any other interface can be used to implement it?

You have two options available to you.

1) __clk_reparent can be used from your .set_rate callback today to
reflect changes made to the tree topology.  OMAP uses this in our PLL
.set_rate implementation: depending on the re-lock frequency the PLL
may switch parents dynamically.  __clk_reparent does the
framework-level cleanup needed for this (that function is also used
when populating the clock tree with new clock nodes).

2) __clk_set_parent could be made non-static if you needed this (I've
been meaning to talk to Saravana about this since I think MSM needs
something like this).  This is not done today but perhaps you can
experiment with it and let me know what you find?

If you stare at the code for a while I'm sure you'll see how it comes
together.  If any infrastructure that you require is missing please
let me know.

Regards,
Mike

> Thanks.
>
> 2012/5/8 Turquette, Mike <mturquette@xxxxxx>:
>> On Mon, May 7, 2012 at 8:39 AM, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote:
>>> On 05/06/2012 06:03 PM, Mike Turquette wrote:
>>>> On 20120503-19:13, Peter De Schrijver wrote:
>>>>> Hi,
>>>>>
>>>>> I started looking into what would be needed to move our tegra30 clock code
>>>>> to the common clock framework. The tegra30 clocktree is rather flat. Basically
>>>>> there are a bunch of sources (13 PLLs, external audio clocks, osc and 32Khz)
>>>>> and peripheral clocks which have a mux (with 4 or more inputs), a divider and
>>>>> a gate. So almost every peripheral clock can have multiple parents.
>>>>>
>>>>> Some questions:
>>>>>
>>>>> 1) should these peripheral clocks be modelled as 3 different clocks
>>>>>    (mux -> divider -> gate) or would it be better to make a new clock type for
>>>>>    this?
>>>>>
>>>>
>>>> That is really for you to decide.  If the semantics of the existing mux,
>>>> divider and gate in drivers/clk/clk-*.c work well for you then I think
>>>> the answer is "yes".  There is infrastructure for register-access
>>>> locking in those common types which might help your complex clocks.
>>>>
>>>> Thanks to the parent rate propagation stuff in clk_set_rate it should be
>>>> possible for your drivers to only be aware of the gate and call
>>>> clk_set_rate on only that clock, which propagates up to the divider and,
>>>> if necessary, again propagates up to the mux.
>>>>
>>>> I encourage you to try that first.  But if you find the semantics of
>>>> those basic clock types aren't cutting it for you then you must create a
>>>> type which is platform-specific.
>>>
>>> A lot of these mux/divider/gate clocks go out to peripherals, whose
>>> drivers want to call both clk_set_rate() and clk_en/disable() on the clock.
>>>
>>> There's only 1 clock reaching the peripheral in HW, so the driver should
>>> only call clk_get() once, and similarly the DT should only provide a
>>> single clock to the driver.
>>>
>>
>> Sounds good so far.
>>
>>> Given the mux->divider->gate clock construction, that clock would
>>> presumably be the gate object. clk_en/disable() clearly work there, but
>>> is clk_set_rate() intended to propagate up the chain until it can be
>>> satisfied, i.e. does the gate clock object's set_rate() op simply call
>>> the parent's set_rate() op?
>>>
>>
>> Only if you set the CLK_SET_RATE_PARENT flag on the children.  Take
>> the following example sub-tree:
>>
>> mux
>>  |
>> div
>>  |
>> gate
>>
>> If gate does NOT have CLK_SET_RATE_PARENT set in struct clk->flags
>> then clk_set_rate will do nothing (since it does not implement a
>> .set_rate callback).  If the flag is set then the framework will kick
>> the rate request up to the parent, 'div'.  Div implements a .set_rate
>> callback and may or may not set the CLK_SET_RATE_PARENT flag.  Having
>> a .set_rate implemenation does preclude one from propagating the rate
>> change up the tree, so it is possible to adjust the divider to a new
>> rate AND still kick the rate change request up to the parent, 'mux'.
>> In your case 'mux' should probably not set CLK_SET_RATE_PARENT and the
>> propagation will end there.  'mux' might implement a re-parenting
>> operation as part of it's .set_rate implementation.
>>
>> This is covered fairly well in Documentation/clk.txt
>>
>>> If the order were instead mux->gate->divider, would it be correct for
>>> enable/disable to propagate from the divider to the gate?
>>>
>>
>> Correct.  Order doesn't matter.  Even in the absence of a
>> .prepare/.enable callback the framework will still propagate these
>> changes as part of typical use-counting rules.
>>
>>>>> 2) how to define the default parent? in many cases the hw reset value isn't
>>>>>    a very sensible choice, so the kernel probably needs to set a parent of
>>>>>    many of them if we don't want to rely on bootloader configuration.
>>>>
>>>> The only related thing handled at the framework level is _discovery_ of
>>>> the parent during clock registration/initialization.  If you don't trust
>>>> the bootloader and want to set things up as soon as possible (a wise
>>>> move) then I suggest you do so from your platform clock code at the same
>>>> time that you register your clocks with the framework.  Something like:
>>>>
>>>>       struct clk *c;
>>>>       c = clk_register(...);
>>>>       if (IS_ERR(c))
>>>>               omg_fail();
>>>>       clk_set_parent(c, b);
>>>>
>>>> Where 'b' is a parent of 'c'.  Register your clock tree top-down and you
>>>> can re-parent as you go.
>>>
>>> I'm hoping we can represent this in device tree somehow, so that
>>> individual boards can set the clock tree up differently depending on
>>> their needs (e.g. Tegra20 doesn't have quite enough PLLs, so sometimes a
>>> particular PLL will be used to generate the 2nd display's pixel clock,
>>> whereas on other boards it may be used for some peripherals). So, we'd
>>> like to describe this in DT.
>>>
>>> It seems like it'd be pretty common to want the kernel to fully
>>> initialize the clock tree, and to do this from device tree, so perhaps
>>> this might evolve into a common (part of) a cross-SoC clock binding, or
>>> some kind of utility function that parsed a clock-tree-init-table from DT?
>>
>> As long as the configuration of your clocks can be expressed by the
>> existing clk api, e.g. clk_prepare / clk_enable / clk_set_rate /
>> clk_set_parent, then I think it would be easy to define a format
>> whereby the clock is configured immediately after being registered.
>> Essentially it would do what I outline in my original email above, but
>> the configuration could be encoded into DT.
>>
>> Any platform-specific clock configuration (bits that aren't obviously
>> expressed by the clk.h api) will be harder to configure through
>> generic DT bindings.
>>
>> Regards,
>> Mike
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> --
> Zhoujie Wu
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux