Re:Re: [PATCH 06/12] clk: mmp: add mmp private gate clock

[Chao Xie]

At 2014-09-04 01:55:37, "Mike Turquette" <mturquette@xxxxxxxxxx> wrote:
>Quoting Chao Xie (2014-08-25 21:38:18)
>> From: Chao Xie <chao.xie@xxxxxxxxxxx>
>> 
>> Some SOCes have this kind of the gate clock
>> 1. There are some bits to control the gate not only one bit.
>> 2. Some clocks has operations of "out of reset" and "enable".
>>    To enable clock, we need do "out of reset" and "enable".
>>    To disable clock, we may not need "set to reset". It depends
>>    on the SOCes' design.
>
>Are there any other IP blocks affected by the "out of reset" and "set to
>reset" states? I wonder if you might benefit from the reset controller
>framework?  For example see,
>
>drivers/clk/qcom/gcc-apq8084.c
>
><snip>

>
Thanks to point it out.
To use the reset framework, there are some problem.
1. the reset bit is combined with the clocks disable/enable bits in same register. Seperating setting them means spinlock
protection and 2 operations for read-update the bits.


except that, i think reset framework can bring some benefits.


Even without the reset bit, we still need the gate clocks.
The enable/disable is not a bit operation, and it is bits operation for some devices.
In fact i want to use it to replace the old clk-apbc and clk-apmu clocks. 

>> +static int mmp_clk_gate_enable(struct clk_hw *hw)
>> +{
>> +       struct mmp_clk_gate *gate = to_clk_mmp_gate(hw);
>> +       struct clk *clk = hw->clk;
>> +       unsigned long flags = 0;
>> +       unsigned long rate;
>> +       u32 tmp;
>> +
>> +       if (gate->lock)
>> +               spin_lock_irqsave(gate->lock, flags);
>> +
>> +       tmp = readl(gate->reg);
>> +       tmp &= ~gate->mask;
>> +       tmp |= gate->val_enable;
>> +       writel(tmp, gate->reg);
>> +
>> +       if (gate->lock)
>> +               spin_unlock_irqrestore(gate->lock, flags);
>> +
>> +       if (gate->flags & MMP_CLK_GATE_NEED_DELAY) {
>> +               rate = __clk_get_rate(clk);
>> +               /* Need delay 2 cycles. */
>> +               udelay(2000000/rate);
>
>How long are these delays? Long enough to warrant using clk_prepare
>instead of clk_enable? Are these clocks enabled from interrupt context?

>
For power optimization, some clocks need to be enabled/disable in interrupt context.
The worst delay is rate=32KHZ, so the delay is 62.5us.

>Regards,
>Mike


N嫥叉靣笡y氊b瞂千v豝?藓{.n?壏{睉赙zXФ洝塄}财爖?j:+v墾?珣赙zZ+€?zf＂穐殘啳嗃i?鄗?畐ア?櫒璀??撷f旟^j谦y呩@A玜囤?0鹅h?鍜i