Hi Kever,
Am Montag, 11. August 2014, 18:03:33 schrieb Kever Yang:
> On 08/09/2014 06:20 AM, Heiko Stübner wrote:
> > Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:
> >> Heiko,
> >>
> >> On Fri, Aug 1, 2014 at 1:15 AM, Heiko Stübner <heiko@xxxxxxxxx> wrote:
> >>> Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:
> >>>> Quoting Heiko Stübner (2014-07-31 16:29:34)
> >>>>
> >>>>> Hi Mike,
> >>>>>
> >>>>> Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:
> >>>>>> Quoting Heiko Stuebner (2014-07-29 12:12:05)
> >>>>>>
> >>>>>>> The clock-tree contains clocks that should never get disabled
> >>>>>>> automatically. One example are the base ACLKs, the base supplies
> >>>>>>> for
> >>>>>>> all
> >>>>>>> peripherals.
> >>>>>>>
> >>>>>>> Therefore add a structure similar to the sunxi clock-tree to
> >>>>>>> protect
> >>>>>>> these
> >>>>>>> special clocks from being disabled.
> >>>>>>>
> >>>>>>> Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
> >>>>>>> ---
> >>>>>>>
> >>>>>>> drivers/clk/rockchip/clk-rk3188.c | 7 +++++++
> >>>>>>> drivers/clk/rockchip/clk-rk3288.c | 7 +++++++
> >>>>>>> drivers/clk/rockchip/clk.c | 13 +++++++++++++
> >>>>>>> drivers/clk/rockchip/clk.h | 1 +
> >>>>>>> 4 files changed, 28 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> b/drivers/clk/rockchip/clk-rk3188.c index a83a6d8..5aef277 100644
> >>>>>>> --- a/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> +++ b/drivers/clk/rockchip/clk-rk3188.c
> >>>>>>> @@ -599,6 +599,11 @@ static struct rockchip_clk_branch
> >>>>>>> rk3188_clk_branches[] __initdata = {>
> >>>>>>>
> >>>>>>> GATE(ACLK_GPS, "aclk_gps", "aclk_peri", 0,
> >>>>>>> RK2928_CLKGATE_CON(8),
> >>>>>>> 13, GFLAGS),>
> >>>>>>>
> >>>>>>> };
> >>>>>>>
> >>>>>>> +static const char *rk3188_critical_clocks[] __initconst = {
> >>>>>>> + "aclk_cpu",
> >>>>>>> + "aclk_peri",
> >>>>>>
> >>>>>> I'm not against the idea of critical clocks, but I want to verify
> >>>>>> that
> >>>>>> there is no other driver out there that is a better fit for claiming
> >>>>>> these clks via clk_get and enabling them the normal way via
> >>>>>> clk_enable?
> >>>>>
> >>>>> In the clock hierarchy of Rockchip SoCs, both aclks listed here, are
> >>>>> sources for pclk and hclk, as well as sourcing some other peripheral
> >>>>> gates further below too. So from what I've seen from the clock
> >>>>> diagrams,
> >>>>> there is nothing that would claim these clocks directly, and it
> >>>>> wouldn't
> >>>>> also make any sense to let them get disabled as there will always be
> >>>>> something using them (for example the dram-controller).
> >>>>
> >>>> Sounds good. Just out of curiosity, under what circumstances would you
> >>>> want to gate them? Is there a use case for it?
> >>>
> >>> hmm, I don't see a use-case for gating these at runtime right now,
> >>> simply
> >>> because there should be a user for them all the time. (both aclks
> >>> combined
> >>> have at least 68 consumers on the rk3288 and a similar number on the
> >>> previous socs)
> >>>
> >>> The only thing I could think of would be something suspend related -
> >>> which
> >>> we don't have yet. But then this would probably happen in the clock
> >>> controller itself anyway in some late suspend-related action, so it
> >>> could
> >>> take into account them being defined as critical clocks.
> >>
> >> I know Rockchip has some funky stuff planned for memory scaling too.
> >> Perhaps Kever can comment whether these two clocks might need to be
> >> disabled in that case?
> >
> > hmm looking at the core clock tree, I wouldn't think so.
> >
> > The only intersection between the ddr-clk, aclk_cpu and aclk_peri is the
> > gpll which can be a source to both. But the ddr-clk is mainly sourced
> > from the dpll anyway.
> >
> > In any case, turning off aclk_cpu/aclk_peri in this scenario wouldn't
> > normally be possible anyway, as most of the time some pclk_* would be
> > active anyway.
> Basically, aclk_cpu/aclk_peri have very little chance to be gated during
> run-time,
> but both of then may be gated when system enter suspend mode.
>
> For aclk_cpu, this clock supplies most of clocks in pd_bus actually,
> some clocks not listed
> as a module clock will be needed, like cpu I/D bus fetch
> instruction/data from dram via
> bus based on aclk_cpu. For this situation, can we use a dummy clock to
> hold the
> aclk_cpu not to be gated at run-time?
>
> For aclk_peri, this clock is able to be gated run-time in theory,
> although it's no use
> in actual system, because we have many devices on this clock and at most
> of the time
> some of then would be active just as you have mentioned.
>
> The system suspend is another scenario, and we tend to gate both of the
> clock if possible,
> can we do that if this patch is applied?
as we will need suspend operations in the clock driver anyway [likely
something like the Samsung clk driver does], it shouldn't be a problem to lift
the hold on the critical clocks when suspending.
Heiko
>
> -Kever
>
> >> In any case, this patch fixes a hang at boot when using the PWM driver
> >> that just landed, so:
> >>
> >> Tested-by: Doug Anderson <dianders@xxxxxxxxxxxx>
> >
> > thanks
> >
> >
> > Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
