Re: [PATCH 1/4] clk: rockchip: protect critical clocks from getting disabled
From: Kever Yang <hidden>
Date: 2014-08-11 10:03:33
Also in:
linux-arm-kernel
On 08/09/2014 06:20 AM, Heiko Stübner wrote:
Am Freitag, 8. August 2014, 14:58:11 schrieb Doug Anderson:quoted
Heiko, On Fri, Aug 1, 2014 at 1:15 AM, Heiko Stübner [off-list ref] wrote:quoted
Am Donnerstag, 31. Juli 2014, 17:30:25 schrieb Mike Turquette:quoted
Quoting Heiko Stübner (2014-07-31 16:29:34)quoted
Hi Mike, Am Donnerstag, 31. Juli 2014, 15:45:23 schrieb Mike Turquette:quoted
Quoting Heiko Stuebner (2014-07-29 12:12:05)quoted
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 <redacted> --- 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.cb/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_branchrk3188_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? -Kever
quoted
In any case, this patch fixes a hang at boot when using the PWM driver that just landed, so: Tested-by: Doug Anderson <redacted>thanks Heiko
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html