[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b7093a17-b30c-cdec-b02c-c6bc5a8d08c9@rock-chips.com>
Date: Wed, 10 Apr 2019 14:23:26 +0800
From: "elaine.zhang" <zhangqing@...k-chips.com>
To: Douglas Anderson <dianders@...omium.org>,
Heiko Stuebner <heiko@...ech.de>
Cc: Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Caesar Wang <caesar.wang@...k-chips.com>,
linux-rockchip@...ts.infradead.org, mka@...omium.org,
ryandcase@...omium.org, linux-clk@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/3] Revert "clk: rockchip: mark noc and some special clk
as critical on rk3288"
hi,
在 2019/4/10 上午4:47, Douglas Anderson 写道:
> This reverts commit 55bb6a633c33caf68ab470907ecf945289cb733d.
>
> The clocks that were enabled by that patch are pretty questionable.
> Specifically looking at what has been shipping on rk3288-veyron
> Chromebooks almost all of these clocks are safely turned off and cause
> no apparent problems. If some boards need these clocks turned on for
> some reason then it seems like we should figure out how to do that at
> a board level.
>
> NOTE: turning these clocks off doesn't seem to do a whole lot in terms
> of power savings (checking the power on the logic rail). It appears
> to save maybe 1-2mW. ...but still it seems like we should turn the
> clocks off if they aren't needed.
>
> Digging into the clocks here to describe why they shouldn't need to be
> left on:
>
> atclk: No documentation about this clock other than that it goes to
> the CPU. CPU functions fine without it on.
>
> jtag: Presumably this clock is only needed if you're debugging with
> JTAG. It doesn't seem like it makes sense to waste power for every
> rk3288 user. Perhaps this could be turned on with a CONFIG option?
>
> pclk_dbg, pclk_core_niu: On veyron Chromebooks we turn these two
> clocks on only during kernel panics in order to access some coresight
> registers. Since nothing in the upstream kernel does this we should
> be able to leave them off safely.
>
> hsicphy12m_xin12m: There is no indication of why this clock would need
> to be turned on for boards that don't use HSIC.
>
> pclk_ddrupctl[0-1], pclk_publ0[0-1]: On veyron Chromebooks we turn
> these 4 clocks on only when doing DDR transitions and they are off
> otherwise. I see no reason why they'd need to be on in the upstream
> kernel which doesn't support DDRFreq.
>
> pmu_hclk_otg0: A "chip design defect" is mentioned in the original
> patch but no details. This clock has always been gated in shipping
> veyron Chromebooks so presumably this chip defect doesn't affect all
> boards.
>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
>
> drivers/clk/rockchip/clk-rk3288.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 5a67b7869960..06287810474e 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -313,13 +313,13 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
> COMPOSITE_NOMUX(0, "aclk_core_mp", "armclk", CLK_IGNORE_UNUSED,
> RK3288_CLKSEL_CON(0), 4, 4, DFLAGS | CLK_DIVIDER_READ_ONLY,
> RK3288_CLKGATE_CON(12), 6, GFLAGS),
> - COMPOSITE_NOMUX(0, "atclk", "armclk", CLK_IGNORE_UNUSED,
> + COMPOSITE_NOMUX(0, "atclk", "armclk", 0,
> RK3288_CLKSEL_CON(37), 4, 5, DFLAGS | CLK_DIVIDER_READ_ONLY,
> RK3288_CLKGATE_CON(12), 7, GFLAGS),
> COMPOSITE_NOMUX(0, "pclk_dbg_pre", "armclk", CLK_IGNORE_UNUSED,
> RK3288_CLKSEL_CON(37), 9, 5, DFLAGS | CLK_DIVIDER_READ_ONLY,
> RK3288_CLKGATE_CON(12), 8, GFLAGS),
> - GATE(0, "pclk_dbg", "pclk_dbg_pre", CLK_IGNORE_UNUSED,
> + GATE(0, "pclk_dbg", "pclk_dbg_pre", 0,
> RK3288_CLKGATE_CON(12), 9, GFLAGS),
> GATE(0, "cs_dbg", "pclk_dbg_pre", CLK_IGNORE_UNUSED,
> RK3288_CLKGATE_CON(12), 10, GFLAGS),
> @@ -647,7 +647,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
> INVERTER(SCLK_HSADC, "sclk_hsadc", "sclk_hsadc_out",
> RK3288_CLKSEL_CON(22), 7, IFLAGS),
>
> - GATE(0, "jtag", "ext_jtag", CLK_IGNORE_UNUSED,
> + GATE(0, "jtag", "ext_jtag", 0,
> RK3288_CLKGATE_CON(4), 14, GFLAGS),
CLK_IGNORE_UNUSED:
Whether to close the unused clk after clk init complete. not affect
there own enable/disable.
JTAG is not have device node, when need jtag to debug, may be the system
is crashed, there is no way to turn on this clk.
>
> COMPOSITE_NODIV(SCLK_USBPHY480M_SRC, "usbphy480m_src", mux_usbphy480m_p, 0,
> @@ -656,7 +656,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
> COMPOSITE_NODIV(SCLK_HSICPHY480M, "sclk_hsicphy480m", mux_hsicphy480m_p, 0,
> RK3288_CLKSEL_CON(29), 0, 2, MFLAGS,
> RK3288_CLKGATE_CON(3), 6, GFLAGS),
> - GATE(0, "hsicphy12m_xin12m", "xin12m", CLK_IGNORE_UNUSED,
> + GATE(0, "hsicphy12m_xin12m", "xin12m", 0,
> RK3288_CLKGATE_CON(13), 9, GFLAGS),
> DIV(0, "hsicphy12m_usbphy", "sclk_hsicphy480m", 0,
> RK3288_CLKSEL_CON(11), 8, 6, DFLAGS),
> @@ -837,12 +837,6 @@ static const char *const rk3288_critical_clocks[] __initconst = {
> "pclk_alive_niu",
> "pclk_pd_pmu",
> "pclk_pmu_niu",
> - "pclk_core_niu",
> - "pclk_ddrupctl0",
> - "pclk_publ0",
> - "pclk_ddrupctl1",
> - "pclk_publ1",
These clks needed enable, device node not use this clk, so we mark it as
critical.
> - "pmu_hclk_otg0",
It's a soc bug, pmu_hclk_otg0 must always on.
> };
>
> static void __iomem *rk3288_cru_base;
Powered by blists - more mailing lists