[PATCH v7 10/11] ARM: hi3xxx: add clk-hi3716
Mike Turquette
mturquette at linaro.org
Thu Aug 22 04:16:48 EDT 2013
Quoting Haojian Zhuang (2013-08-22 00:50:52)
> On 22 August 2013 13:59, Mike Turquette <mturquette at linaro.org> wrote:
> > Quoting zhangfei gao (2013-08-21 18:19:33)
> >> On Thu, Aug 22, 2013 at 5:43 AM, Mike Turquette <mturquette at linaro.org> wrote:
> >> > Quoting Haojian Zhuang (2013-08-19 19:31:12)
> >> >> From: Zhangfei Gao <zhangfei.gao at linaro.org>
> >> >>
> >> >> Signed-off-by: Zhangfei Gao <zhangfei.gao at linaro.org>
> >> >> Signed-off-by: Zhang Mingjun <zhang.mingjun at linaro.org>
> >> >> ---
> >>
> >> >> +static int hi3716_clkgate_prepare(struct clk_hw *hw)
> >> >> +{
> >> >> + struct hi3716_clk *clk = to_clk_hi3716(hw);
> >> >> + unsigned long flags = 0;
> >> >> + u32 reg;
> >> >> +
> >> >> + spin_lock_irqsave(&_lock, flags);
> >> >> +
> >> >> + reg = readl_relaxed(clk->reg);
> >> >> + reg &= ~BIT(clk->reset_bit);
> >> >> + writel_relaxed(reg, clk->reg);
> >> >> +
> >> >> + spin_unlock_irqrestore(&_lock, flags);
> >> >> +
> >> >> + return 0;
> >> >> +}
> >> >> +
> >> >> +static void hi3716_clkgate_unprepare(struct clk_hw *hw)
> >> >> +{
> >> >> + struct hi3716_clk *clk = to_clk_hi3716(hw);
> >> >> + unsigned long flags = 0;
> >> >> + u32 reg;
> >> >> +
> >> >> + spin_lock_irqsave(&_lock, flags);
> >> >> +
> >> >> + reg = readl_relaxed(clk->reg);
> >> >> + reg |= BIT(clk->reset_bit);
> >> >> + writel_relaxed(reg, clk->reg);
> >> >> +
> >> >> + spin_unlock_irqrestore(&_lock, flags);
> >> >> +}
> >> >> +
> >> >> +static struct clk_ops hi3716_clkgate_ops = {
> >> >> + .prepare = hi3716_clkgate_prepare,
> >> >> + .unprepare = hi3716_clkgate_unprepare,
> >> >> +};
> >> >
> >> > Why .prepare & .unprepare instead of .enable & .disable?
> >> >
> >> > Regards,
> >> > Mike
> >>
> >> Thanks Mike for the review
> >>
> >> the .enable & .disable is directly use clk_gate_ops.
> >>
> >> + hi3716_clkgate_ops.enable = clk_gate_ops.enable;
> >> + hi3716_clkgate_ops.disable = clk_gate_ops.disable;
> >> + hi3716_clkgate_ops.is_enabled = clk_gate_ops.is_enabled;
> >> + p_clk->gate.bit_idx = array[1];
> >>
> >> prepare & unprepare is handle reset bit, while enable & disable is
> >> handle enable bit.
> >> We have to extend since clk_gate_ops does not consider prepare &
> >> unprepare, otherwise it would be simpler.
> >
> > I understand why you made this choice from the perspective of re-using
> > the existing gate-clock implementation. What I meant in my question is
> > whether or not handling the reset bit in the .prepare/.unprepare is the
> > right thing.
> >
> > For instance if you called clk_enable or clk_disable from within an
> > interrupt handler would you want to toggle the reset bit in that
> > instance?
> >
>
> No. We don't want to access reset bit for clk_enable() & clk_disable().
>
> We don't know what issue will occur if we always control reset & unreset with
> enabling/disabling.
Ok then your use of .prepare & .unprepare sounds correct to me.
Thanks,
Mike
>
> Regards
> Haojian
More information about the linux-arm-kernel
mailing list