moving Tegra30 to the common clock framework

[zhoujie wu]

Hi Mike,
Could you please explain more details about how to implement a
re-parenting operation as part of it's .set_rate implementation?
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.
Any other interface can be used to implement it?
Thanks.

2012/5/8 Turquette, Mike <mturquette at ti.com>:
> On Mon, May 7, 2012 at 8:39 AM, Stephen Warren <swarren at wwwdotorg.org> 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Zhoujie Wu