Messages in this thread | | | From | Anson Huang <> | Subject | RE: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array | Date | Tue, 14 Aug 2018 07:31:26 +0000 |
| |
Hi, Peng
Anson Huang Best Regards!
> -----Original Message----- > From: Peng Fan > Sent: Monday, August 13, 2018 9:16 AM > To: Anson Huang <anson.huang@nxp.com>; shawnguo@kernel.org; > s.hauer@pengutronix.de; kernel@pengutronix.de; Fabio Estevam > <fabio.estevam@nxp.com>; mturquette@baylibre.com; sboyd@kernel.org; > linux-arm-kernel@lists.infradead.org; linux-clk@vger.kernel.org; > linux-kernel@vger.kernel.org; Rob Herring <robh@kernel.org> > Cc: dl-linux-imx <linux-imx@nxp.com> > Subject: RE: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array > > Hi Anson, > > > > > -----Original Message----- > > > > From: Anson Huang > > > > Sent: 2018年8月8日 12:39 > > > > To: shawnguo@kernel.org; s.hauer@pengutronix.de; > > > > kernel@pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>; > > > > mturquette@baylibre.com; sboyd@kernel.org; > > > > linux-arm-kernel@lists.infradead.org; > > > > linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org > > > > Cc: dl-linux-imx <linux-imx@nxp.com> > > > > Subject: [PATCH 2/2] clk: imx: imx7d: remove clks_init_on array > > > > > > > > Clock framework will enable those clocks registered with > > > > CLK_IS_CRITICAL flag, so no need to have clks_init_on array during > > > > clock > > > initialization now. > > > > > > Will it be more flexible to parse dts saying "critical-clocks = <xxx>" > > > or "init-on-arrary=<xxx>" > > > and enable those clocks? > > > > Parsing the clocks arrays from dtb is another way of enabling critical > > clocks, but for current i.MX6/7 platforms, we implement it in same way > > as most of other SoCs, currently I did NOT see any necessity of > > putting them in dtb, just adding flag during clock registering is more > > simple, if there is any special requirement for different clocks set > > to be enabled, then we can add support to enable the method of parsing > critical-clocks from dtb. Just my two cents. > > Thinking about OP-TEE want to use one device, but it's clocks are registered by > Linux, because there is no module in Linux side use it, it will shutdown the clock, > which cause OP-TEE could not access the device. > > Then people have to modify clk code to add CLK_IS_CRITICAL flag to make sure > the clocks are not shutdown by Linux. > > However adding a new property in clk node and let driver code parse the dts, > there is no need to modify clk driver code when OP-TEE needs another device > clock. For supporting OP-TEE or other features which has special requirement about clock settings, I think we can have another patch to add parsing critical clocks from dtb in common place of i.MX clock drivers.
Anson.
> > Regards, > Peng. > > > > > Anson. > > > > > > > > Regards, > > > Peng. > > > > > > > > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > > > --- > > > > drivers/clk/imx/clk-imx7d.c | 27 ++++++++------------------- > > > > drivers/clk/imx/clk.h | 7 +++++++ > > > > 2 files changed, 15 insertions(+), 19 deletions(-) > > > > > > > > diff --git a/drivers/clk/imx/clk-imx7d.c > > > > b/drivers/clk/imx/clk-imx7d.c index c4518d7..076460b 100644 > > > > --- a/drivers/clk/imx/clk-imx7d.c > > > > +++ b/drivers/clk/imx/clk-imx7d.c > > > > @@ -379,13 +379,6 @@ static const char *pll_enet_bypass_sel[] = { > > > > "pll_enet_main", "pll_enet_main_src static const char > > > > *pll_audio_bypass_sel[] = { "pll_audio_main", > > > > "pll_audio_main_src", }; static const char *pll_video_bypass_sel[] > > > > = { "pll_video_main", "pll_video_main_src", }; > > > > > > > > -static int const clks_init_on[] __initconst = { > > > > - IMX7D_ARM_A7_ROOT_CLK, IMX7D_MAIN_AXI_ROOT_CLK, > > > > - IMX7D_PLL_SYS_MAIN_480M_CLK, IMX7D_IPG_ROOT_CLK, > > > > - IMX7D_DRAM_PHYM_ROOT_CLK, IMX7D_DRAM_ROOT_CLK, > > > > - IMX7D_DRAM_PHYM_ALT_ROOT_CLK, > IMX7D_DRAM_ALT_ROOT_CLK, > > > > -}; > > > > - > > > > static struct clk_onecell_data clk_data; > > > > > > > > static struct clk ** const uart_clks[] __initconst = { @@ -403,7 > > > > +396,6 @@ static void __init imx7d_clocks_init(struct device_node > > > *ccm_node) { > > > > struct device_node *np; > > > > void __iomem *base; > > > > - int i; > > > > > > > > clks[IMX7D_CLK_DUMMY] = imx_clk_fixed("dummy", 0); > > > > clks[IMX7D_OSC_24M_CLK] = of_clk_get_by_name(ccm_node, > "osc"); > > > @@ > > > > -466,7 +458,7 @@ static void __init imx7d_clocks_init(struct > > > > device_node > > > > *ccm_node) > > > > clks[IMX7D_PLL_SYS_MAIN_120M] = > > > > imx_clk_fixed_factor("pll_sys_main_120m", "pll_sys_main_clk", 1, 4); > > > > clks[IMX7D_PLL_DRAM_MAIN_533M] = > > > > imx_clk_fixed_factor("pll_dram_533m", "pll_dram_main_clk", 1, 2); > > > > > > > > - clks[IMX7D_PLL_SYS_MAIN_480M_CLK] = > > > > imx_clk_gate_dis("pll_sys_main_480m_clk", "pll_sys_main_480m", > > > > base > > > > + 0xb0, 4); > > > > + clks[IMX7D_PLL_SYS_MAIN_480M_CLK] = > > > > +imx_clk_gate_dis_flags("pll_sys_main_480m_clk", > > > > +"pll_sys_main_480m", base + 0xb0, 4, CLK_IS_CRITICAL); > > > > clks[IMX7D_PLL_SYS_MAIN_240M_CLK] = > > > > imx_clk_gate_dis("pll_sys_main_240m_clk", "pll_sys_main_240m", > > > > base > > > > + 0xb0, 5); > > > > clks[IMX7D_PLL_SYS_MAIN_120M_CLK] = > > > > imx_clk_gate_dis("pll_sys_main_120m_clk", "pll_sys_main_120m", > > > > base > > > > + 0xb0, 6); > > > > clks[IMX7D_PLL_DRAM_MAIN_533M_CLK] = > > > > imx_clk_gate("pll_dram_533m_clk", "pll_dram_533m", base + 0x70, > > > > 12); @@ > > > > -719,7 +711,7 @@ static void __init imx7d_clocks_init(struct > > > > device_node > > > > *ccm_node) > > > > clks[IMX7D_ENET_AXI_ROOT_DIV] = > > > > imx_clk_divider2("enet_axi_post_div", "enet_axi_pre_div", base + > > > > 0x8900, 0, > > > 6); > > > > clks[IMX7D_NAND_USDHC_BUS_ROOT_CLK] = > > > > imx_clk_divider2("nand_usdhc_root_clk", "nand_usdhc_pre_div", base > > > > + 0x8980, 0, 6); > > > > clks[IMX7D_AHB_CHANNEL_ROOT_DIV] = > > > > imx_clk_divider2("ahb_root_clk", "ahb_pre_div", base + 0x9000, 0, 6); > > > > - clks[IMX7D_IPG_ROOT_CLK] = imx_clk_divider2("ipg_root_clk", > > > > "ahb_root_clk", base + 0x9080, 0, 2); > > > > + clks[IMX7D_IPG_ROOT_CLK] = imx_clk_divider_flags("ipg_root_clk", > > > > +"ahb_root_clk", base + 0x9080, 0, 2, CLK_IS_CRITICAL | > > > > +CLK_OPS_PARENT_ENABLE | CLK_SET_RATE_PARENT); > > > > clks[IMX7D_DRAM_ROOT_DIV] = imx_clk_divider2("dram_post_div", > > > > "dram_cg", base + 0x9880, 0, 3); > > > > clks[IMX7D_DRAM_PHYM_ALT_ROOT_DIV] = > > > > imx_clk_divider2("dram_phym_alt_post_div", > > > > "dram_phym_alt_pre_div", base > > > > + 0xa000, 0, 3); > > > > clks[IMX7D_DRAM_ALT_ROOT_DIV] = > > > > imx_clk_divider2("dram_alt_post_div", "dram_alt_pre_div", base + > > > > 0xa080, 0, 3); @@ -783,17 +775,17 @@ static void __init > > > > imx7d_clocks_init(struct device_node *ccm_node) > > > > clks[IMX7D_CLKO1_ROOT_DIV] = imx_clk_divider2("clko1_post_div", > > > > "clko1_pre_div", base + 0xbd80, 0, 6); > > > > clks[IMX7D_CLKO2_ROOT_DIV] = imx_clk_divider2("clko2_post_div", > > > > "clko2_pre_div", base + 0xbe00, 0, 6); > > > > > > > > - clks[IMX7D_ARM_A7_ROOT_CLK] = > imx_clk_gate4("arm_a7_root_clk", > > > > "arm_a7_div", base + 0x4000, 0); > > > > + clks[IMX7D_ARM_A7_ROOT_CLK] = > > > > imx_clk_gate2_flags("arm_a7_root_clk", > > > > +"arm_a7_div", base + 0x4000, 0, CLK_IS_CRITICAL | > > > > +CLK_OPS_PARENT_ENABLE); > > > > clks[IMX7D_ARM_M4_ROOT_CLK] = > > imx_clk_gate4("arm_m4_root_clk", > > > > "arm_m4_div", base + 0x4010, 0); > > > > - clks[IMX7D_MAIN_AXI_ROOT_CLK] = > imx_clk_gate4("main_axi_root_clk", > > > > "axi_post_div", base + 0x4040, 0); > > > > + clks[IMX7D_MAIN_AXI_ROOT_CLK] = > > > > +imx_clk_gate2_flags("main_axi_root_clk", "axi_post_div", base + > > > > +0x4040, 0, CLK_IS_CRITICAL | CLK_OPS_PARENT_ENABLE); > > > > clks[IMX7D_DISP_AXI_ROOT_CLK] = > > imx_clk_gate4("disp_axi_root_clk", > > > > "disp_axi_post_div", base + 0x4050, 0); > > > > clks[IMX7D_ENET_AXI_ROOT_CLK] = > > imx_clk_gate4("enet_axi_root_clk", > > > > "enet_axi_post_div", base + 0x4060, 0); > > > > clks[IMX7D_OCRAM_CLK] = imx_clk_gate4("ocram_clk", > > > > "main_axi_root_clk", base + 0x4110, 0); > > > > clks[IMX7D_OCRAM_S_CLK] = imx_clk_gate4("ocram_s_clk", > > > > "ahb_root_clk", base + 0x4120, 0); > > > > - clks[IMX7D_DRAM_ROOT_CLK] = imx_clk_gate4("dram_root_clk", > > > > "dram_post_div", base + 0x4130, 0); > > > > - clks[IMX7D_DRAM_PHYM_ROOT_CLK] = > > > > imx_clk_gate4("dram_phym_root_clk", "dram_phym_cg", base + 0x4130, > > 0); > > > > - clks[IMX7D_DRAM_PHYM_ALT_ROOT_CLK] = > > > > imx_clk_gate4("dram_phym_alt_root_clk", "dram_phym_alt_post_div", > > > > base > > > > + 0x4130, 0); > > > > - clks[IMX7D_DRAM_ALT_ROOT_CLK] = > > > imx_clk_gate4("dram_alt_root_clk", > > > > "dram_alt_post_div", base + 0x4130, 0); > > > > + clks[IMX7D_DRAM_ROOT_CLK] = > > imx_clk_gate2_flags("dram_root_clk", > > > > "dram_post_div", base + 0x4130, 0, CLK_IS_CRITICAL | > > > > CLK_OPS_PARENT_ENABLE); > > > > + clks[IMX7D_DRAM_PHYM_ROOT_CLK] = > > > > imx_clk_gate2_flags("dram_phym_root_clk", "dram_phym_cg", base + > > > > 0x4130, 0, CLK_IS_CRITICAL | CLK_OPS_PARENT_ENABLE); > > > > + clks[IMX7D_DRAM_PHYM_ALT_ROOT_CLK] = > > > > imx_clk_gate2_flags("dram_phym_alt_root_clk", > > > > "dram_phym_alt_post_div", base + 0x4130, 0, CLK_IS_CRITICAL | > > > > CLK_OPS_PARENT_ENABLE); > > > > + clks[IMX7D_DRAM_ALT_ROOT_CLK] = > > > > +imx_clk_gate2_flags("dram_alt_root_clk", "dram_alt_post_div", > > > > +base > > > > ++ 0x4130, 0, CLK_IS_CRITICAL | CLK_OPS_PARENT_ENABLE); > > > > clks[IMX7D_OCOTP_CLK] = imx_clk_gate4("ocotp_clk", > > "ipg_root_clk", > > > > base + 0x4230, 0); > > > > clks[IMX7D_SNVS_CLK] = imx_clk_gate4("snvs_clk", "ipg_root_clk", > > > > base + 0x4250, 0); > > > > clks[IMX7D_MU_ROOT_CLK] = imx_clk_gate4("mu_root_clk", > > > > "ipg_root_clk", base + 0x4270, 0); @@ -882,9 +874,6 @@ static void > > > > __init imx7d_clocks_init(struct device_node *ccm_node) > > > > clk_data.clk_num = ARRAY_SIZE(clks); > > > > of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data); > > > > > > > > - for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) > > > > - clk_prepare_enable(clks[clks_init_on[i]]); > > > > - > > > > clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], > > > > clks[IMX7D_PLL_ARM_MAIN]); > > > > clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], > > > > clks[IMX7D_PLL_DRAM_MAIN]); > > > > clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], > > > > clks[IMX7D_PLL_SYS_MAIN]); diff --git a/drivers/clk/imx/clk.h > > > > b/drivers/clk/imx/clk.h index 8076ec0..5895e223 100644 > > > > --- a/drivers/clk/imx/clk.h > > > > +++ b/drivers/clk/imx/clk.h > > > > @@ -137,6 +137,13 @@ static inline struct clk > > > > *imx_clk_gate_dis(const char *name, const char *parent, > > > > shift, CLK_GATE_SET_TO_DISABLE, &imx_ccm_lock); } > > > > > > > > +static inline struct clk *imx_clk_gate_dis_flags(const char > > > > +*name, const char > > > > *parent, > > > > + void __iomem *reg, u8 shift, unsigned long flags) { > > > > + return clk_register_gate(NULL, name, parent, flags | > > > > CLK_SET_RATE_PARENT, reg, > > > > + shift, CLK_GATE_SET_TO_DISABLE, &imx_ccm_lock); } > > > > + > > > > static inline struct clk *imx_clk_gate2(const char *name, const > > > > char > > > *parent, > > > > void __iomem *reg, u8 shift) > > > > { > > > > -- > > > > 2.7.4
| |