On 12/02, Bai Ping wrote: > diff --git a/drivers/clk/imx/clk-imx6sll.c b/drivers/clk/imx/clk-imx6sll.c > new file mode 100644 > index 0000000..c5219e1 > --- /dev/null > +++ b/drivers/clk/imx/clk-imx6sll.c > @@ -0,0 +1,366 @@ > +/* > + * Copyright (C) 2016 Freescale Semiconductor, Inc. > + * > + * The code contained herein is licensed under the GNU General Public > + * License. You may obtain a copy of the GNU General Public License > + * Version 2 or later at the following locations: > + * > + * http://www.opensource.org/licenses/gpl-license.html > + * http://www.gnu.org/copyleft/gpl.html > + */ > + > +#include <dt-bindings/clock/imx6sll-clock.h> > +#include <linux/clk.h> > +#include <linux/clkdev.h> Is this include used? > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/types.h> Is there an include of clk_provider.h missing? > + > +#include "clk.h" > + > +#define BM_CCM_CCDR_MMDC_CH0_MASK (0x2 << 16) > +#define CCDR 0x4 > + > +static const char *pll_bypass_src_sels[] = { "osc", "dummy", }; const char * const? For all of these names. > +static const char *pll1_bypass_sels[] = { "pll1", "pll1_bypass_src", }; > +static const char *pll2_bypass_sels[] = { "pll2", "pll2_bypass_src", }; > +static const char *pll3_bypass_sels[] = { "pll3", "pll3_bypass_src", }; > +static const char *pll4_bypass_sels[] = { "pll4", "pll4_bypass_src", }; > +static const char *pll5_bypass_sels[] = { "pll5", "pll5_bypass_src", }; > +static const char *pll6_bypass_sels[] = { "pll6", "pll6_bypass_src", }; > +static const char *pll7_bypass_sels[] = { "pll7", "pll7_bypass_src", }; > +static const char *step_sels[] = { "osc", "pll2_pfd2_396m", }; > +static const char *pll1_sw_sels[] = { "pll1_sys", "step", }; > +static const char *axi_alt_sels[] = { "pll2_pfd2_396m", "pll3_pfd1_540m", }; > +static const char *axi_sels[] = {"periph", "axi_alt_sel", }; > +static const char *periph_pre_sels[] = { "pll2_bus", "pll2_pfd2_396m", "pll2_pfd0_352m", "pll2_198m", }; > +static const char *periph2_pre_sels[] = { "pll2_bus", "pll2_pfd2_396m", "pll2_pfd0_352m", "pll4_audio_div", }; > +static const char *periph_clk2_sels[] = { "pll3_usb_otg", "osc", "osc", }; > +static const char *periph2_clk2_sels[] = { "pll3_usb_otg", "osc", }; > +static const char *periph_sels[] = { "periph_pre", "periph_clk2", }; > +static const char *periph2_sels[] = { "periph2_pre", "periph2_clk2", }; > +static const char *usdhc_sels[] = { "pll2_pfd2_396m", "pll2_pfd0_352m", }; > +static const char *ssi_sels[] = {"pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_audio_div", "dummy",}; > +static const char *spdif_sels[] = { "pll4_audio_div", "pll3_pfd2_508m", "pll5_video_div", "pll3_usb_otg", }; > +static const char *ldb_di0_div_sels[] = { "ldb_di0_div_3_5", "ldb_di0_div_7", }; > +static const char *ldb_di1_div_sels[] = { "ldb_di1_div_3_5", "ldb_di1_div_7", }; > +static const char *ldb_di0_sels[] = { "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll2_pfd3_594m", "pll2_pfd1_594m", "pll3_pfd3_454m", }; > +static const char *ldb_di1_sels[] = { "pll3_usb_otg", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll2_bus", "pll3_pfd3_454m", "pll3_pfd2_508m", }; > +static const char *lcdif_pre_sels[] = { "pll2_bus", "pll3_pfd3_454m", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd1_594m", "pll3_pfd1_540m", }; > +static const char *ecspi_sels[] = { "pll3_60m", "osc", }; > +static const char *uart_sels[] = { "pll3_80m", "osc", }; > +static const char *perclk_sels[] = { "ipg", "osc", }; > +static const char *lcdif_sels[] = { "lcdif_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", }; > + > +static const char *epdc_pre_sels[] = { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd2_508m", }; > +static const char *epdc_sels[] = { "epdc_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", }; > + > +static struct clk *clks[IMX6SLL_CLK_END]; > +static struct clk_onecell_data clk_data; > + > +static int const clks_init_on[] __initconst = { static const int is more preferred style. > + IMX6SLL_CLK_AIPSTZ1, IMX6SLL_CLK_AIPSTZ2, > + IMX6SLL_CLK_OCRAM, IMX6SLL_CLK_ARM, IMX6SLL_CLK_ROM, > + IMX6SLL_CLK_MMDC_P0_FAST, IMX6SLL_CLK_MMDC_P0_IPG, > +}; > + > +static struct clk_div_table post_div_table[] = { const? > + { .val = 2, .div = 1, }, > + { .val = 1, .div = 2, }, > + { .val = 0, .div = 4, }, > + { } > +}; > + > +static struct clk_div_table video_div_table[] = { const? > + { .val = 0, .div = 1, }, > + { .val = 1, .div = 2, }, > + { .val = 2, .div = 1, }, > + { .val = 3, .div = 4, }, > + { } > +}; > + > +static u32 share_count_audio; > +static u32 share_count_ssi1; > +static u32 share_count_ssi2; > +static u32 share_count_ssi3; > + > +static void __init imx6sll_clocks_init(struct device_node *ccm_node) > +{ > + struct device_node *np; > + void __iomem *base; > + int i; > + > + clks[IMX6SLL_CLK_DUMMY] = imx_clk_fixed("dummy", 0); > + > + clks[IMX6SLL_CLK_CKIL] = of_clk_get_by_name(ccm_node, "ckil"); > + clks[IMX6SLL_CLK_OSC] = of_clk_get_by_name(ccm_node, "osc"); > + > + /* ipp_di clock is external input */ > + clks[IMX6SLL_CLK_IPP_DI0] = of_clk_get_by_name(ccm_node, "ipp_di0"); > + clks[IMX6SLL_CLK_IPP_DI1] = of_clk_get_by_name(ccm_node, "ipp_di1"); > + > + np = of_find_compatible_node(NULL, NULL, "fsl,imx6sll-anatop"); > + base = of_iomap(np, 0); > + WARN_ON(!base); > + > + clks[IMX6SLL_PLL1_BYPASS_SRC] = imx_clk_mux("pll1_bypass_src", base + 0x00, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels)); > + clks[IMX6SLL_PLL2_BYPASS_SRC] = imx_clk_mux("pll2_bypass_src", base + 0x30, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels)); > + clks[IMX6SLL_PLL3_BYPASS_SRC] = imx_clk_mux("pll3_bypass_src", base + 0x10, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels)); > + clks[IMX6SLL_PLL4_BYPASS_SRC] = imx_clk_mux("pll4_bypass_src", base + 0x70, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels)); > + clks[IMX6SLL_PLL5_BYPASS_SRC] = imx_clk_mux("pll5_bypass_src", base + 0xa0, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels)); > + clks[IMX6SLL_PLL6_BYPASS_SRC] = imx_clk_mux("pll6_bypass_src", base + 0xe0, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels)); > + clks[IMX6SLL_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", base + 0x20, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels)); > + > + clks[IMX6SLL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS, "pll1", "pll1_bypass_src", base + 0x00, 0x7f); > + clks[IMX6SLL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", base + 0x30, 0x1); > + clks[IMX6SLL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB, "pll3", "pll3_bypass_src", base + 0x10, 0x3); > + clks[IMX6SLL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV, "pll4", "pll4_bypass_src", base + 0x70, 0x7f); > + clks[IMX6SLL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV, "pll5", "pll5_bypass_src", base + 0xa0, 0x7f); > + clks[IMX6SLL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET, "pll6", "pll6_bypass_src", base + 0xe0, 0x3); > + clks[IMX6SLL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB, "pll7", "pll7_bypass_src", base + 0x20, 0x3); > + > + clks[IMX6SLL_PLL1_BYPASS] = imx_clk_mux_flags("pll1_bypass", base + 0x00, 16, 1, pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels), CLK_SET_RATE_PARENT); > + clks[IMX6SLL_PLL2_BYPASS] = imx_clk_mux_flags("pll2_bypass", base + 0x30, 16, 1, pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels), CLK_SET_RATE_PARENT); > + clks[IMX6SLL_PLL3_BYPASS] = imx_clk_mux_flags("pll3_bypass", base + 0x10, 16, 1, pll3_bypass_sels, ARRAY_SIZE(pll3_bypass_sels), CLK_SET_RATE_PARENT); > + clks[IMX6SLL_PLL4_BYPASS] = imx_clk_mux_flags("pll4_bypass", base + 0x70, 16, 1, pll4_bypass_sels, ARRAY_SIZE(pll4_bypass_sels), CLK_SET_RATE_PARENT); > + clks[IMX6SLL_PLL5_BYPASS] = imx_clk_mux_flags("pll5_bypass", base + 0xa0, 16, 1, pll5_bypass_sels, ARRAY_SIZE(pll5_bypass_sels), CLK_SET_RATE_PARENT); > + clks[IMX6SLL_PLL6_BYPASS] = imx_clk_mux_flags("pll6_bypass", base + 0xe0, 16, 1, pll6_bypass_sels, ARRAY_SIZE(pll6_bypass_sels), CLK_SET_RATE_PARENT); > + clks[IMX6SLL_PLL7_BYPASS] = imx_clk_mux_flags("pll7_bypass", base + 0x20, 16, 1, pll7_bypass_sels, ARRAY_SIZE(pll7_bypass_sels), CLK_SET_RATE_PARENT); > + > + /* Do not bypass PLLs initially */ > + clk_set_parent(clks[IMX6SLL_PLL1_BYPASS], clks[IMX6SLL_CLK_PLL1]); > + clk_set_parent(clks[IMX6SLL_PLL2_BYPASS], clks[IMX6SLL_CLK_PLL2]); > + clk_set_parent(clks[IMX6SLL_PLL3_BYPASS], clks[IMX6SLL_CLK_PLL3]); > + clk_set_parent(clks[IMX6SLL_PLL4_BYPASS], clks[IMX6SLL_CLK_PLL4]); > + clk_set_parent(clks[IMX6SLL_PLL5_BYPASS], clks[IMX6SLL_CLK_PLL5]); > + clk_set_parent(clks[IMX6SLL_PLL6_BYPASS], clks[IMX6SLL_CLK_PLL6]); > + clk_set_parent(clks[IMX6SLL_PLL7_BYPASS], clks[IMX6SLL_CLK_PLL7]); Can we just put raw register writes here instead? I'd prefer we didn't use clk consumer APIs to do things to the clk tree from the providers. The problem there being that: 1) We're trying to move away from using consumer APIs in provider drivers. It's ok if they're used during probe, but inside clk_ops is not preferred. 2) Even if you have a clk pointer, it may be "orphaned" at the time of registration and so calling the APIs here works now but eventually we may want to return an EPROBE_DEFER error in that case and this may block that effort. I suppose if this is the only clk driver on this machine then this last point isn't a concern and things are probably ok here. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
