Quoting Taniya Das (2018-06-04 00:56:25)Unused ones I would clean up.
diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-sdm845.c
new file mode 100644
index 0000000..317ab33
--- /dev/null
+++ b/drivers/clk/qcom/dispcc-sdm845.c
@@ -0,0 +1,679 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+#include <dt-bindings/clock/qcom,dispcc-sdm845.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "clk-regmap-divider.h"
Used?
Sure, will move this in a header and submit the patch.+#include "common.h"
+#include "gdsc.h"
+#include "reset.h"
+
+#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
We really need to move this into the header file.
$ git grep '#define F(' -- drivers/clk/qcom/ | wc -l
13
I would move it.+[...]
+static const char * const disp_cc_parent_names_4[] = {
+ "bi_tcxo",
+ "dsi0_phy_pll_out_dsiclk",
+ "dsi1_phy_pll_out_dsiclk",
+ "core_bi_pll_test_se",
+};
+
+static const struct alpha_pll_config disp_cc_pll0_config = {
+ .l = 0x2c,
+ .alpha = 0xcaaa,
+};
Any reason this can't be put on the stack in the probe function?
+[...]
+static struct clk_alpha_pll disp_cc_pll0 = {
+ .offset = 0x0,
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
+ .clkr = {
+ .hw.init = &(struct clk_init_data){
+ .name = "disp_cc_pll0",
+ .parent_names = (const char *[]){ "bi_tcxo" },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_fabia_ops,
+ },
+ },
+};
+
+static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
+ .cmd_rcgr = 0x2058,
+ .mnd_width = 8,
+ .hid_width = 5,
+ .parent_map = disp_cc_parent_map_4,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "disp_cc_mdss_pclk0_clk_src",
+ .parent_names = disp_cc_parent_names_4,
+ .num_parents = 4,
+ .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
Why is the nocache flag needed? Applies to all clks in this file.
The actual end is larger than this :( , I have listed the range till the register offset are used here.+ .ops = &clk_pixel_ops,[...]
+ },
+};
+
+static struct clk_rcg2 disp_cc_mdss_pclk1_clk_src = {
+ .cmd_rcgr = 0x2070,
+ .mnd_width = 8,
+ .hid_width = 5,
+ .parent_map = disp_cc_parent_map_4,
+ .clkr.hw.init = &(struct clk_init_data){
+
+static const struct regmap_config disp_cc_sdm845_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x10000,
This seems arbitrarily large. List the actual end?
These are used for deciding whether to enable HW based Clock gating or not. Setting these bit will enable HW based Clock gating.+ .fast_io = true,
+};
+
+static const struct qcom_cc_desc disp_cc_sdm845_desc = {
+ .config = &disp_cc_sdm845_regmap_config,
+ .clks = disp_cc_sdm845_clocks,
+ .num_clks = ARRAY_SIZE(disp_cc_sdm845_clocks),
+ .resets = disp_cc_sdm845_resets,
+ .num_resets = ARRAY_SIZE(disp_cc_sdm845_resets),
+ .gdscs = disp_cc_sdm845_gdscs,
+ .num_gdscs = ARRAY_SIZE(disp_cc_sdm845_gdscs),
+};
+
+static const struct of_device_id disp_cc_sdm845_match_table[] = {
+ { .compatible = "qcom,sdm845-dispcc" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, disp_cc_sdm845_match_table);
+
+static int disp_cc_sdm845_probe(struct platform_device *pdev)
+{
+ struct regmap *regmap;
+
+ regmap = qcom_cc_map(pdev, &disp_cc_sdm845_desc);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ clk_fabia_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
+
+ /* Enable clock gating for DSI and MDP clocks */
Hardware clock gating? What does this mean.
+ regmap_update_bits(regmap, 0x8000, 0x7f0, 0x7f0);
+
+ return qcom_cc_really_probe(pdev, &disp_cc_sdm845_desc, regmap);
+}