Quoting Amit Nischal (2018-07-23 04:26:33) > diff --git a/drivers/clk/qcom/camcc-sdm845.c b/drivers/clk/qcom/camcc-sdm845.c > new file mode 100644 > index 0000000..61e5ec2 > --- /dev/null > +++ b/drivers/clk/qcom/camcc-sdm845.c > @@ -0,0 +1,1736 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + */ > + > +#include <linux/clk.h> Is this include used? > +#include <linux/clk-provider.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#include <dt-bindings/clock/qcom,camcc-sdm845.h> > + > +#include "common.h" > +#include "clk-alpha-pll.h" > +#include "clk-branch.h" > +#include "clk-rcg.h" > +#include "clk-regmap.h" > +#include "gdsc.h" > + > +enum { > + P_BI_TCXO, > + P_CAM_CC_PLL0_OUT_EVEN, > + P_CAM_CC_PLL1_OUT_EVEN, > + P_CAM_CC_PLL2_OUT_EVEN, > + P_CAM_CC_PLL3_OUT_EVEN, > + P_CORE_BI_PLL_TEST_SE, > +}; > + > +static const struct parent_map cam_cc_parent_map_0[] = { > + { P_BI_TCXO, 0 }, > + { P_CAM_CC_PLL2_OUT_EVEN, 1 }, > + { P_CAM_CC_PLL1_OUT_EVEN, 2 }, > + { P_CAM_CC_PLL3_OUT_EVEN, 5 }, > + { P_CAM_CC_PLL0_OUT_EVEN, 6 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const char * const cam_cc_parent_names_0[] = { > + "bi_tcxo", > + "cam_cc_pll2_out_even", > + "cam_cc_pll1_out_even", > + "cam_cc_pll3_out_even", > + "cam_cc_pll0_out_even", > + "core_bi_pll_test_se", > +}; > + > +static struct clk_alpha_pll cam_cc_pll0 = { > + .offset = 0x0, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr = { > + .hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll0", > + .parent_names = (const char *[]){ "bi_tcxo" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_fabia_ops, > + }, > + }, > +}; > + > +static const struct clk_div_table post_div_table_fabia_even[] = { > + { 0x0, 1 }, > + { 0x1, 2 }, > + { } > +}; > + > +static struct clk_alpha_pll_postdiv cam_cc_pll0_out_even = { > + .offset = 0x0, > + .post_div_shift = 8, > + .post_div_table = post_div_table_fabia_even, > + .num_post_div = ARRAY_SIZE(post_div_table_fabia_even), > + .width = 4, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll0_out_even", > + .parent_names = (const char *[]){ "cam_cc_pll0" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_postdiv_fabia_ops, > + }, > +}; > + > +static struct clk_alpha_pll cam_cc_pll1 = { > + .offset = 0x1000, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr = { > + .hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll1", > + .parent_names = (const char *[]){ "bi_tcxo" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_fabia_ops, > + }, > + }, > +}; > + > +static struct clk_alpha_pll_postdiv cam_cc_pll1_out_even = { > + .offset = 0x1000, > + .post_div_shift = 8, > + .post_div_table = post_div_table_fabia_even, > + .num_post_div = ARRAY_SIZE(post_div_table_fabia_even), > + .width = 4, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll1_out_even", > + .parent_names = (const char *[]){ "cam_cc_pll1" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_postdiv_fabia_ops, > + }, > +}; > + > +static struct clk_alpha_pll cam_cc_pll2 = { > + .offset = 0x2000, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr = { > + .hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll2", > + .parent_names = (const char *[]){ "bi_tcxo" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_fabia_ops, > + }, > + }, > +}; > + > +static struct clk_alpha_pll_postdiv cam_cc_pll2_out_even = { > + .offset = 0x2000, > + .post_div_shift = 8, > + .post_div_table = post_div_table_fabia_even, > + .num_post_div = ARRAY_SIZE(post_div_table_fabia_even), > + .width = 4, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll2_out_even", > + .parent_names = (const char *[]){ "cam_cc_pll2" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_postdiv_fabia_ops, > + }, > +}; > + > +static struct clk_alpha_pll cam_cc_pll3 = { > + .offset = 0x3000, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr = { > + .hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll3", > + .parent_names = (const char *[]){ "bi_tcxo" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_fabia_ops, > + }, > + }, > +}; > + > +static struct clk_alpha_pll_postdiv cam_cc_pll3_out_even = { > + .offset = 0x3000, > + .post_div_shift = 8, > + .post_div_table = post_div_table_fabia_even, > + .num_post_div = ARRAY_SIZE(post_div_table_fabia_even), > + .width = 4, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA], > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_pll3_out_even", > + .parent_names = (const char *[]){ "cam_cc_pll3" }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_postdiv_fabia_ops, > + }, > +}; > + > +static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { > + F(19200000, P_BI_TCXO, 1, 0, 0), > + F(100000000, P_CAM_CC_PLL0_OUT_EVEN, 6, 0, 0), > + F(200000000, P_CAM_CC_PLL0_OUT_EVEN, 3, 0, 0), > + F(404000000, P_CAM_CC_PLL1_OUT_EVEN, 2, 0, 0), > + F(480000000, P_CAM_CC_PLL2_OUT_EVEN, 1, 0, 0), > + F(600000000, P_CAM_CC_PLL0_OUT_EVEN, 1, 0, 0), > + { } > +}; > + > +static struct clk_rcg2 cam_cc_bps_clk_src = { > + .cmd_rcgr = 0x600c, > + .mnd_width = 0, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_0, > + .freq_tbl = ftbl_cam_cc_bps_clk_src, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_bps_clk_src", > + .parent_names = cam_cc_parent_names_0, > + .num_parents = 6, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_shared_ops, Why are shared ops used in this driver? > + }, > +}; > + > +static const struct freq_tbl ftbl_cam_cc_cci_clk_src[] = { > + F(19200000, P_BI_TCXO, 1, 0, 0), > + F(37500000, P_CAM_CC_PLL0_OUT_EVEN, 16, 0, 0), > + F(50000000, P_CAM_CC_PLL0_OUT_EVEN, 12, 0, 0), > + F(100000000, P_CAM_CC_PLL0_OUT_EVEN, 6, 0, 0), > + { } > +}; > + [...] > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_mclk0_clk_src", > + .parent_names = cam_cc_parent_names_0, > + .num_parents = 6, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_ops, > + }, > +}; > + > +static struct clk_rcg2 cam_cc_mclk1_clk_src = { > + .cmd_rcgr = 0x4024, > + .mnd_width = 8, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_0, > + .freq_tbl = ftbl_cam_cc_mclk0_clk_src, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_mclk1_clk_src", > + .parent_names = cam_cc_parent_names_0, > + .num_parents = 6, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_ops, > + }, > +}; > + > +static struct clk_rcg2 cam_cc_mclk2_clk_src = { > + .cmd_rcgr = 0x4044, > + .mnd_width = 8, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_0, > + .freq_tbl = ftbl_cam_cc_mclk0_clk_src, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_mclk2_clk_src", > + .parent_names = cam_cc_parent_names_0, > + .num_parents = 6, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_ops, > + }, > +}; > + > +static struct clk_rcg2 cam_cc_mclk3_clk_src = { > + .cmd_rcgr = 0x4064, > + .mnd_width = 8, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_0, > + .freq_tbl = ftbl_cam_cc_mclk0_clk_src, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_mclk3_clk_src", > + .parent_names = cam_cc_parent_names_0, > + .num_parents = 6, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_ops, > + }, > +}; > + > +static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { > + F(19200000, P_BI_TCXO, 1, 0, 0), > + F(60000000, P_CAM_CC_PLL0_OUT_EVEN, 10, 0, 0), > + F(66666667, P_CAM_CC_PLL0_OUT_EVEN, 9, 0, 0), > + F(73846154, P_CAM_CC_PLL2_OUT_EVEN, 6.5, 0, 0), > + F(80000000, P_CAM_CC_PLL2_OUT_EVEN, 6, 0, 0), > + { } > +}; > + > +static struct clk_rcg2 cam_cc_slow_ahb_clk_src = { > + .cmd_rcgr = 0x6054, > + .mnd_width = 0, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_0, > + .freq_tbl = ftbl_cam_cc_slow_ahb_clk_src, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "cam_cc_slow_ahb_clk_src", > + .parent_names = cam_cc_parent_names_0, > + .num_parents = 6, > + .flags = CLK_SET_RATE_PARENT, Is CLK_SET_RATE_PARENT intentionally set on these RCGs so that they can reconfigure the PLL frequency? Wouldn't that be a fixed rate PLL frequency? > + .ops = &clk_rcg2_ops, > + }, > +}; > + [...] > + > +static int cam_cc_sdm845_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap; > + struct alpha_pll_config cam_cc_pll_config = { }; > + > + regmap = qcom_cc_map(pdev, &cam_cc_sdm845_desc); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + cam_cc_pll_config.l = 0x1f, > + cam_cc_pll_config.alpha = 0x4000, Replace these commas with semicolons. > + clk_fabia_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll_config); > + > + cam_cc_pll_config.l = 0x2a, > + cam_cc_pll_config.alpha = 0x1556, > + clk_fabia_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll_config); All over this function. Can you also add comments in the commit text and code describing why oddities like CLK_SET_RATE_PARENT on RCGs and shared rcg ops is used? -- To unsubscribe from this list: send the line "unsubscribe linux-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html