[PATCH 0/2] Add display clock controller driver for SDM845

[PATCH 0/2] Add display clock controller driver for SDM845

[Taniya Das]

Add support for the display clock controller found on SDM845
based devices. This would allow display drivers to probe and
control their clocks.

Taniya Das (2):
  dt-bindings: clock: Introduce QCOM Display clock bindings
  clk: qcom: Add display clock controller driver for SDM845

 .../devicetree/bindings/clock/qcom,dispcc.txt      |  18 +
 drivers/clk/qcom/Kconfig                           |  10 +
 drivers/clk/qcom/Makefile                          |   1 +
 drivers/clk/qcom/dispcc-sdm845.c                   | 704 +++++++++++++++++++++
 include/dt-bindings/clock/qcom,dispcc-sdm845.h     |  42 ++
 5 files changed, 775 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc.txt
 create mode 100644 drivers/clk/qcom/dispcc-sdm845.c
 create mode 100644 include/dt-bindings/clock/qcom,dispcc-sdm845.h

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

[PATCH 1/2] dt-bindings: clock: Introduce QCOM Display clock bindings

[Taniya Das]

Add device tree bindings for display clock controller for Qualcomm
Technology Inc's SoCs.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 .../devicetree/bindings/clock/qcom,dispcc.txt      | 19 +++++++++
 include/dt-bindings/clock/qcom,dispcc-sdm845.h     | 45 ++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc.txt
 create mode 100644 include/dt-bindings/clock/qcom,dispcc-sdm845.h

diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc.txt b/Documentation/devicetree/bindings/clock/qcom,dispcc.txt
new file mode 100644
index 0000000..d639e18
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc.txt
@@ -0,0 +1,19 @@
+Qualcomm Technologies, Inc. Display Clock Controller Binding
+------------------------------------------------------------
+
+Required properties :
+
+- compatible : shall contain "qcom,sdm845-dispcc"
+- reg : shall contain base register location and length.
+- #clock-cells : from common clock binding, shall contain 1.
+- #reset-cells : from common reset binding, shall contain 1.
+- #power-domain-cells : from generic power domain binding, shall contain 1.
+
+Example:
+	dispcc: clock-controller@af00000 {
+		compatible = "qcom,sdm845-dispcc";
+		reg = <0xaf00000 0x100000>;
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+		#power-domain-cells = <1>;
+	};
diff --git a/include/dt-bindings/clock/qcom,dispcc-sdm845.h b/include/dt-bindings/clock/qcom,dispcc-sdm845.h
new file mode 100644
index 0000000..11eed4b
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,dispcc-sdm845.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_SDM_DISP_CC_SDM845_H
+#define _DT_BINDINGS_CLK_SDM_DISP_CC_SDM845_H
+
+/* DISP_CC clock registers */
+#define DISP_CC_MDSS_AHB_CLK					0
+#define DISP_CC_MDSS_AXI_CLK					1
+#define DISP_CC_MDSS_BYTE0_CLK					2
+#define DISP_CC_MDSS_BYTE0_CLK_SRC				3
+#define DISP_CC_MDSS_BYTE0_INTF_CLK				4
+#define DISP_CC_MDSS_BYTE1_CLK					5
+#define DISP_CC_MDSS_BYTE1_CLK_SRC				6
+#define DISP_CC_MDSS_BYTE1_INTF_CLK				7
+#define DISP_CC_MDSS_ESC0_CLK					8
+#define DISP_CC_MDSS_ESC0_CLK_SRC				9
+#define DISP_CC_MDSS_ESC1_CLK					10
+#define DISP_CC_MDSS_ESC1_CLK_SRC				11
+#define DISP_CC_MDSS_MDP_CLK					12
+#define DISP_CC_MDSS_MDP_CLK_SRC				13
+#define DISP_CC_MDSS_MDP_LUT_CLK				14
+#define DISP_CC_MDSS_PCLK0_CLK					15
+#define DISP_CC_MDSS_PCLK0_CLK_SRC				16
+#define DISP_CC_MDSS_PCLK1_CLK					17
+#define DISP_CC_MDSS_PCLK1_CLK_SRC				18
+#define DISP_CC_MDSS_ROT_CLK					19
+#define DISP_CC_MDSS_ROT_CLK_SRC				20
+#define DISP_CC_MDSS_RSCC_AHB_CLK				21
+#define DISP_CC_MDSS_RSCC_VSYNC_CLK				22
+#define DISP_CC_MDSS_VSYNC_CLK					23
+#define DISP_CC_MDSS_VSYNC_CLK_SRC				24
+#define DISP_CC_PLL0						25
+#define DISP_CC_MDSS_BYTE0_DIV_CLK_SRC				26
+#define DISP_CC_MDSS_BYTE1_DIV_CLK_SRC				27
+
+/* DISP_CC Reset */
+#define DISP_CC_MDSS_RSCC_BCR					0
+
+/* DISP_CC GDSCR */
+#define MDSS_GDSC						0
+
+#endif
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

[PATCH 2/2] clk: qcom: Add display clock controller driver for SDM845

[Taniya Das]

Add support for the display clock controller found on SDM845
based devices. This would allow display drivers to probe and
control their clocks.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/Kconfig         |  10 +
 drivers/clk/qcom/Makefile        |   1 +
 drivers/clk/qcom/dispcc-sdm845.c | 679 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 690 insertions(+)
 create mode 100644 drivers/clk/qcom/dispcc-sdm845.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index e06e1f8..d9e7e75 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -227,6 +227,16 @@ config SDM_GCC_845
 	  Say Y if you want to use peripheral devices such as UART, SPI,
 	  I2C, USB, UFS, SDDC, PCIe, etc.

+config SDM_DISPCC_845
+	tristate "SDM845 Display Clock Controller"
+	select SDM_GCC_845
+	depends on COMMON_CLK_QCOM
+	help
+	  Support for the display clock controller on Qualcomm Technologies, Inc
+	  sdm845 devices.
+	  Say Y if you want to support display devices and functionality such as
+	  splash screen.
+
 config SPMI_PMIC_CLKDIV
 	tristate "SPMI PMIC clkdiv Support"
 	depends on (COMMON_CLK_QCOM && SPMI) || COMPILE_TEST
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 82070e0..96142ff 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -38,4 +38,5 @@ obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o
 obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
 obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
 obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o
+obj-$(CONFIG_SDM_DISPCC_845) += dispcc-sdm845.o
 obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
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"
+#include "common.h"
+#include "gdsc.h"
+#include "reset.h"
+
+#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
+
+enum {
+	P_BI_TCXO,
+	P_CORE_BI_PLL_TEST_SE,
+	P_DISP_CC_PLL0_OUT_MAIN,
+	P_DSI0_PHY_PLL_OUT_BYTECLK,
+	P_DSI0_PHY_PLL_OUT_DSICLK,
+	P_DSI1_PHY_PLL_OUT_BYTECLK,
+	P_DSI1_PHY_PLL_OUT_DSICLK,
+	P_GPLL0_OUT_MAIN,
+	P_GPLL0_OUT_MAIN_DIV,
+};
+
+static const struct parent_map disp_cc_parent_map_0[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_DSI0_PHY_PLL_OUT_BYTECLK, 1 },
+	{ P_DSI1_PHY_PLL_OUT_BYTECLK, 2 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char * const disp_cc_parent_names_0[] = {
+	"bi_tcxo",
+	"dsi0_phy_pll_out_byteclk",
+	"dsi1_phy_pll_out_byteclk",
+	"core_bi_pll_test_se",
+};
+
+static const struct parent_map disp_cc_parent_map_2[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char * const disp_cc_parent_names_2[] = {
+	"bi_tcxo",
+	"core_bi_pll_test_se",
+};
+
+static const struct parent_map disp_cc_parent_map_3[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_DISP_CC_PLL0_OUT_MAIN, 1 },
+	{ P_GPLL0_OUT_MAIN, 4 },
+	{ P_GPLL0_OUT_MAIN_DIV, 5 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char * const disp_cc_parent_names_3[] = {
+	"bi_tcxo",
+	"disp_cc_pll0",
+	"gcc_disp_gpll0_clk_src",
+	"gcc_disp_gpll0_div_clk_src",
+	"core_bi_pll_test_se",
+};
+
+static const struct parent_map disp_cc_parent_map_4[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
+	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+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,
+};
+
+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_byte0_clk_src = {
+	.cmd_rcgr = 0x20d0,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_0,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_byte0_clk_src",
+		.parent_names = disp_cc_parent_names_0,
+		.num_parents = 4,
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+		.ops = &clk_byte2_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_byte1_clk_src = {
+	.cmd_rcgr = 0x20ec,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_0,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_byte1_clk_src",
+		.parent_names = disp_cc_parent_names_0,
+		.num_parents = 4,
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+		.ops = &clk_byte2_ops,
+	},
+};
+
+static const struct freq_tbl ftbl_disp_cc_mdss_esc0_clk_src[] = {
+	F(19200000, P_BI_TCXO, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_esc0_clk_src = {
+	.cmd_rcgr = 0x2108,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_0,
+	.freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_esc0_clk_src",
+		.parent_names = disp_cc_parent_names_0,
+		.num_parents = 4,
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_esc1_clk_src = {
+	.cmd_rcgr = 0x2120,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_0,
+	.freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_esc1_clk_src",
+		.parent_names = disp_cc_parent_names_0,
+		.num_parents = 4,
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static const struct freq_tbl ftbl_disp_cc_mdss_mdp_clk_src[] = {
+	F(19200000, P_BI_TCXO, 1, 0, 0),
+	F(85714286, P_GPLL0_OUT_MAIN, 7, 0, 0),
+	F(100000000, P_GPLL0_OUT_MAIN, 6, 0, 0),
+	F(150000000, P_GPLL0_OUT_MAIN, 4, 0, 0),
+	F(171428571, P_GPLL0_OUT_MAIN, 3.5, 0, 0),
+	F(200000000, P_GPLL0_OUT_MAIN, 3, 0, 0),
+	F(300000000, P_GPLL0_OUT_MAIN, 2, 0, 0),
+	F(344000000, P_DISP_CC_PLL0_OUT_MAIN, 2.5, 0, 0),
+	F(430000000, P_DISP_CC_PLL0_OUT_MAIN, 2, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
+	.cmd_rcgr = 0x2088,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_3,
+	.freq_tbl = ftbl_disp_cc_mdss_mdp_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_mdp_clk_src",
+		.parent_names = disp_cc_parent_names_3,
+		.num_parents = 5,
+		.ops = &clk_rcg2_shared_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,
+		.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){
+		.name = "disp_cc_mdss_pclk1_clk_src",
+		.parent_names = disp_cc_parent_names_4,
+		.num_parents = 4,
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+		.ops = &clk_pixel_ops,
+	},
+};
+
+static const struct freq_tbl ftbl_disp_cc_mdss_rot_clk_src[] = {
+	F(19200000, P_BI_TCXO, 1, 0, 0),
+	F(171428571, P_GPLL0_OUT_MAIN, 3.5, 0, 0),
+	F(300000000, P_GPLL0_OUT_MAIN, 2, 0, 0),
+	F(344000000, P_DISP_CC_PLL0_OUT_MAIN, 2.5, 0, 0),
+	F(430000000, P_DISP_CC_PLL0_OUT_MAIN, 2, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_rot_clk_src = {
+	.cmd_rcgr = 0x20a0,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_3,
+	.freq_tbl = ftbl_disp_cc_mdss_rot_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_rot_clk_src",
+		.parent_names = disp_cc_parent_names_3,
+		.num_parents = 5,
+		.ops = &clk_rcg2_shared_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
+	.cmd_rcgr = 0x20b8,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_2,
+	.freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_vsync_clk_src",
+		.parent_names = disp_cc_parent_names_2,
+		.num_parents = 2,
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static struct clk_branch disp_cc_mdss_ahb_clk = {
+	.halt_reg = 0x4004,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x4004,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_ahb_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_axi_clk = {
+	.halt_reg = 0x4008,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x4008,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_axi_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_byte0_clk = {
+	.halt_reg = 0x2028,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2028,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_byte0_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_byte0_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
+	.reg = 0x20e8,
+	.shift = 0,
+	.width = 2,
+	.clkr = {
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_byte0_div_clk_src",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_byte0_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_GET_RATE_NOCACHE,
+			.ops = &clk_regmap_div_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_byte0_intf_clk = {
+	.halt_reg = 0x202c,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x202c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_byte0_intf_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_byte0_div_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_byte1_clk = {
+	.halt_reg = 0x2030,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2030,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_byte1_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_byte1_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_regmap_div disp_cc_mdss_byte1_div_clk_src = {
+	.reg = 0x2104,
+	.shift = 0,
+	.width = 2,
+	.clkr = {
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_byte1_div_clk_src",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_byte1_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_GET_RATE_NOCACHE,
+			.ops = &clk_regmap_div_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_byte1_intf_clk = {
+	.halt_reg = 0x2034,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2034,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_byte1_intf_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_byte1_div_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_esc0_clk = {
+	.halt_reg = 0x2038,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2038,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_esc0_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_esc0_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_esc1_clk = {
+	.halt_reg = 0x203c,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x203c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_esc1_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_esc1_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_mdp_clk = {
+	.halt_reg = 0x200c,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x200c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_mdp_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_mdp_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_mdp_lut_clk = {
+	.halt_reg = 0x201c,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x201c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_mdp_lut_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_mdp_clk_src",
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_pclk0_clk = {
+	.halt_reg = 0x2004,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2004,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_pclk0_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_pclk0_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_pclk1_clk = {
+	.halt_reg = 0x2008,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2008,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_pclk1_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_pclk1_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_rot_clk = {
+	.halt_reg = 0x2014,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2014,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_rot_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_rot_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_rscc_ahb_clk = {
+	.halt_reg = 0x5004,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x5004,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_rscc_ahb_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_rscc_vsync_clk = {
+	.halt_reg = 0x5008,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x5008,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_rscc_vsync_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_vsync_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_vsync_clk = {
+	.halt_reg = 0x2024,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2024,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_vsync_clk",
+			.parent_names = (const char *[]){
+				"disp_cc_mdss_vsync_clk_src",
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct gdsc mdss_gdsc = {
+	.gdscr = 0x3000,
+	.pd = {
+		.name = "mdss_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = HW_CTRL | POLL_CFG_GDSCR,
+};
+
+static struct clk_regmap *disp_cc_sdm845_clocks[] = {
+	[DISP_CC_MDSS_AHB_CLK] = &disp_cc_mdss_ahb_clk.clkr,
+	[DISP_CC_MDSS_AXI_CLK] = &disp_cc_mdss_axi_clk.clkr,
+	[DISP_CC_MDSS_BYTE0_CLK] = &disp_cc_mdss_byte0_clk.clkr,
+	[DISP_CC_MDSS_BYTE0_CLK_SRC] = &disp_cc_mdss_byte0_clk_src.clkr,
+	[DISP_CC_MDSS_BYTE0_INTF_CLK] = &disp_cc_mdss_byte0_intf_clk.clkr,
+	[DISP_CC_MDSS_BYTE0_DIV_CLK_SRC] =
+					&disp_cc_mdss_byte0_div_clk_src.clkr,
+	[DISP_CC_MDSS_BYTE1_CLK] = &disp_cc_mdss_byte1_clk.clkr,
+	[DISP_CC_MDSS_BYTE1_CLK_SRC] = &disp_cc_mdss_byte1_clk_src.clkr,
+	[DISP_CC_MDSS_BYTE1_INTF_CLK] = &disp_cc_mdss_byte1_intf_clk.clkr,
+	[DISP_CC_MDSS_BYTE1_DIV_CLK_SRC] =
+					&disp_cc_mdss_byte1_div_clk_src.clkr,
+	[DISP_CC_MDSS_ESC0_CLK] = &disp_cc_mdss_esc0_clk.clkr,
+	[DISP_CC_MDSS_ESC0_CLK_SRC] = &disp_cc_mdss_esc0_clk_src.clkr,
+	[DISP_CC_MDSS_ESC1_CLK] = &disp_cc_mdss_esc1_clk.clkr,
+	[DISP_CC_MDSS_ESC1_CLK_SRC] = &disp_cc_mdss_esc1_clk_src.clkr,
+	[DISP_CC_MDSS_MDP_CLK] = &disp_cc_mdss_mdp_clk.clkr,
+	[DISP_CC_MDSS_MDP_CLK_SRC] = &disp_cc_mdss_mdp_clk_src.clkr,
+	[DISP_CC_MDSS_MDP_LUT_CLK] = &disp_cc_mdss_mdp_lut_clk.clkr,
+	[DISP_CC_MDSS_PCLK0_CLK] = &disp_cc_mdss_pclk0_clk.clkr,
+	[DISP_CC_MDSS_PCLK0_CLK_SRC] = &disp_cc_mdss_pclk0_clk_src.clkr,
+	[DISP_CC_MDSS_PCLK1_CLK] = &disp_cc_mdss_pclk1_clk.clkr,
+	[DISP_CC_MDSS_PCLK1_CLK_SRC] = &disp_cc_mdss_pclk1_clk_src.clkr,
+	[DISP_CC_MDSS_ROT_CLK] = &disp_cc_mdss_rot_clk.clkr,
+	[DISP_CC_MDSS_ROT_CLK_SRC] = &disp_cc_mdss_rot_clk_src.clkr,
+	[DISP_CC_MDSS_RSCC_AHB_CLK] = &disp_cc_mdss_rscc_ahb_clk.clkr,
+	[DISP_CC_MDSS_RSCC_VSYNC_CLK] = &disp_cc_mdss_rscc_vsync_clk.clkr,
+	[DISP_CC_MDSS_VSYNC_CLK] = &disp_cc_mdss_vsync_clk.clkr,
+	[DISP_CC_MDSS_VSYNC_CLK_SRC] = &disp_cc_mdss_vsync_clk_src.clkr,
+	[DISP_CC_PLL0] = &disp_cc_pll0.clkr,
+};
+
+static const struct qcom_reset_map disp_cc_sdm845_resets[] = {
+	[DISP_CC_MDSS_RSCC_BCR] = { 0x5000 },
+};
+
+static struct gdsc *disp_cc_sdm845_gdscs[] = {
+	[MDSS_GDSC] = &mdss_gdsc,
+};
+
+static const struct regmap_config disp_cc_sdm845_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= 0x10000,
+	.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 */
+	regmap_update_bits(regmap, 0x8000, 0x7f0, 0x7f0);
+
+	return qcom_cc_really_probe(pdev, &disp_cc_sdm845_desc, regmap);
+}
+
+static struct platform_driver disp_cc_sdm845_driver = {
+	.probe		= disp_cc_sdm845_probe,
+	.driver		= {
+		.name	= "disp_cc-sdm845",
+		.of_match_table = disp_cc_sdm845_match_table,
+	},
+};
+
+static int __init disp_cc_sdm845_init(void)
+{
+	return platform_driver_register(&disp_cc_sdm845_driver);
+}
+subsys_initcall(disp_cc_sdm845_init);
+
+static void __exit disp_cc_sdm845_exit(void)
+{
+	platform_driver_unregister(&disp_cc_sdm845_driver);
+}
+module_exit(disp_cc_sdm845_exit);
+
+MODULE_LICENSE("GPL v2");
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

Re: [PATCH 2/2] clk: qcom: Add display clock controller driver for SDM845

[Stephen Boyd]

Quoting Taniya Das (2018-06-04 00:56:25)
> 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?

> +#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

> +
[...]
> +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.

> +               .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?

> +       .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);
> +}

Re: [PATCH 2/2] clk: qcom: Add display clock controller driver for SDM845

[Stephen Boyd]

Quoting Taniya Das (2018-06-04 00:56:25)
> 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?

> +#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

> +
[...]
> +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.

> +               .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?

> +       .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);
> +}

Re: [PATCH 2/2] clk: qcom: Add display clock controller driver for SDM845

[Stephen Boyd]

Quoting Taniya Das (2018-06-04 00:56:25)
> diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-s=
dm845.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?

> +#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

> +
[...]
> +static const char * const disp_cc_parent_names_4[] =3D {
> +       "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 =3D {
> +       .l =3D 0x2c,
> +       .alpha =3D 0xcaaa,
> +};

Any reason this can't be put on the stack in the probe function?

> +
> +static struct clk_alpha_pll disp_cc_pll0 =3D {
> +       .offset =3D 0x0,
> +       .regs =3D clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
> +       .clkr =3D {
> +               .hw.init =3D &(struct clk_init_data){
> +                       .name =3D "disp_cc_pll0",
> +                       .parent_names =3D (const char *[]){ "bi_tcxo" },
> +                       .num_parents =3D 1,
> +                       .ops =3D &clk_alpha_pll_fabia_ops,
> +               },
> +       },
> +};
[...]
> +
> +static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src =3D {
> +       .cmd_rcgr =3D 0x2058,
> +       .mnd_width =3D 8,
> +       .hid_width =3D 5,
> +       .parent_map =3D disp_cc_parent_map_4,
> +       .clkr.hw.init =3D &(struct clk_init_data){
> +               .name =3D "disp_cc_mdss_pclk0_clk_src",
> +               .parent_names =3D disp_cc_parent_names_4,
> +               .num_parents =3D 4,
> +               .flags =3D CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,

Why is the nocache flag needed? Applies to all clks in this file.

> +               .ops =3D &clk_pixel_ops,
> +       },
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_pclk1_clk_src =3D {
> +       .cmd_rcgr =3D 0x2070,
> +       .mnd_width =3D 8,
> +       .hid_width =3D 5,
> +       .parent_map =3D disp_cc_parent_map_4,
> +       .clkr.hw.init =3D &(struct clk_init_data){
[...]
> +
> +static const struct regmap_config disp_cc_sdm845_regmap_config =3D {
> +       .reg_bits       =3D 32,
> +       .reg_stride     =3D 4,
> +       .val_bits       =3D 32,
> +       .max_register   =3D 0x10000,

This seems arbitrarily large. List the actual end?

> +       .fast_io        =3D true,
> +};
> +
> +static const struct qcom_cc_desc disp_cc_sdm845_desc =3D {
> +       .config =3D &disp_cc_sdm845_regmap_config,
> +       .clks =3D disp_cc_sdm845_clocks,
> +       .num_clks =3D ARRAY_SIZE(disp_cc_sdm845_clocks),
> +       .resets =3D disp_cc_sdm845_resets,
> +       .num_resets =3D ARRAY_SIZE(disp_cc_sdm845_resets),
> +       .gdscs =3D disp_cc_sdm845_gdscs,
> +       .num_gdscs =3D ARRAY_SIZE(disp_cc_sdm845_gdscs),
> +};
> +
> +static const struct of_device_id disp_cc_sdm845_match_table[] =3D {
> +       { .compatible =3D "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 =3D 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_conf=
ig);
> +
> +       /* 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);
> +}

Re: [PATCH 1/2] dt-bindings: clock: Introduce QCOM Display clock bindings

[Stephen Boyd]

Quoting Taniya Das (2018-06-04 00:56:24)
> Add device tree bindings for display clock controller for Qualcomm
> Technology Inc's SoCs.

SDM845 SoCs?

Re: [PATCH 1/2] dt-bindings: clock: Introduce QCOM Display clock bindings

[Stephen Boyd]

Quoting Taniya Das (2018-06-04 00:56:24)
> Add device tree bindings for display clock controller for Qualcomm
> Technology Inc's SoCs.

SDM845 SoCs?

Re: [PATCH 1/2] dt-bindings: clock: Introduce QCOM Display clock bindings

[Rob Herring]

On Mon, Jun 04, 2018 at 01:26:24PM +0530, Taniya Das wrote:
> Add device tree bindings for display clock controller for Qualcomm
> Technology Inc's SoCs.
> 
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---
>  .../devicetree/bindings/clock/qcom,dispcc.txt      | 19 +++++++++
>  include/dt-bindings/clock/qcom,dispcc-sdm845.h     | 45 ++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc.txt
>  create mode 100644 include/dt-bindings/clock/qcom,dispcc-sdm845.h

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/2] clk: qcom: Add display clock controller driver for SDM845

[Taniya Das]

Hello Stephen,

Thanks for review.

On 6/12/2018 1:25 PM, Stephen Boyd wrote:
> Quoting Taniya Das (2018-06-04 00:56:25)
>> 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?
>
Unused ones I would clean up.

>> +#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
> 
Sure, will move this in a header and submit the patch.

>> +
> [...]
>> +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?
> 
I would move it.
>> +
>> +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.
> 

This flag is required for all RCGs whose PLLs are controlled outside the 
clock controller. The display code would require the recalculated rate 
always.

>> +               .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?
> 
The actual end is larger than this :( , I have listed the range till the 
register offset are used here.

>> +       .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.
> 
These are used for deciding whether to enable HW based Clock gating or 
not. Setting these bit will enable HW based Clock gating.

>> +       regmap_update_bits(regmap, 0x8000, 0x7f0, 0x7f0);
>> +
>> +       return qcom_cc_really_probe(pdev, &disp_cc_sdm845_desc, regmap);
>> +}

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [PATCH 2/2] clk: qcom: Add display clock controller driver for SDM845

[Stephen Boyd]

Quoting Taniya Das (2018-06-13 02:18:59)
> Hello Stephen,
> 
> Thanks for review.
> 
> On 6/12/2018 1:25 PM, Stephen Boyd wrote:
> > Quoting Taniya Das (2018-06-04 00:56:25)
> >> 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
> 
> >> +#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
> > 
> Sure, will move this in a header and submit the patch.

Thanks!

> >> +
> >> +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.
> > 
> 
> This flag is required for all RCGs whose PLLs are controlled outside the 
> clock controller. The display code would require the recalculated rate 
> always.

That doesn't really answer the question. It seems to describe some sort
of problem with the PLL being controlled outside the clk controller/clk
framework. Please add more details and comments into the code 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?
> > 
> The actual end is larger than this :( , I have listed the range till the 
> register offset are used here.

Ok.

> 
> >> +       .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.
> > 
> These are used for deciding whether to enable HW based Clock gating or 
> not. Setting these bit will enable HW based Clock gating.

Please state it's hardware clock gating then in the comment.

> 
> >> +       regmap_update_bits(regmap, 0x8000, 0x7f0, 0x7f0);
> >> +
> >> +       return qcom_cc_really_probe(pdev, &disp_cc_sdm845_desc, regmap);
> >> +}
> 

Re: [PATCH 2/2] clk: qcom: Add display clock controller driver for SDM845

[Stephen Boyd]

Quoting Taniya Das (2018-06-13 02:18:59)
> Hello Stephen,
> =

> Thanks for review.
> =

> On 6/12/2018 1:25 PM, Stephen Boyd wrote:
> > Quoting Taniya Das (2018-06-04 00:56:25)
> >> diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispc=
c-sdm845.c
> >> new file mode 100644
> >> index 0000000..317ab33
> >> --- /dev/null
> >> +++ b/drivers/clk/qcom/dispcc-sdm845.c
> =

> >> +#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
> > =

> Sure, will move this in a header and submit the patch.

Thanks!

> >> +
> >> +static struct clk_alpha_pll disp_cc_pll0 =3D {
> >> +       .offset =3D 0x0,
> >> +       .regs =3D clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
> >> +       .clkr =3D {
> >> +               .hw.init =3D &(struct clk_init_data){
> >> +                       .name =3D "disp_cc_pll0",
> >> +                       .parent_names =3D (const char *[]){ "bi_tcxo" =
},
> >> +                       .num_parents =3D 1,
> >> +                       .ops =3D &clk_alpha_pll_fabia_ops,
> >> +               },
> >> +       },
> >> +};
> > [...]
> >> +
> >> +static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src =3D {
> >> +       .cmd_rcgr =3D 0x2058,
> >> +       .mnd_width =3D 8,
> >> +       .hid_width =3D 5,
> >> +       .parent_map =3D disp_cc_parent_map_4,
> >> +       .clkr.hw.init =3D &(struct clk_init_data){
> >> +               .name =3D "disp_cc_mdss_pclk0_clk_src",
> >> +               .parent_names =3D disp_cc_parent_names_4,
> >> +               .num_parents =3D 4,
> >> +               .flags =3D CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> > =

> > Why is the nocache flag needed? Applies to all clks in this file.
> > =

> =

> This flag is required for all RCGs whose PLLs are controlled outside the =

> clock controller. The display code would require the recalculated rate =

> always.

That doesn't really answer the question. It seems to describe some sort
of problem with the PLL being controlled outside the clk controller/clk
framework. Please add more details and comments into the code here.

> =

> >> +               .ops =3D &clk_pixel_ops,
> >> +       },
> >> +};
> >> +
> >> +static struct clk_rcg2 disp_cc_mdss_pclk1_clk_src =3D {
> >> +       .cmd_rcgr =3D 0x2070,
> >> +       .mnd_width =3D 8,
> >> +       .hid_width =3D 5,
> >> +       .parent_map =3D disp_cc_parent_map_4,
> >> +       .clkr.hw.init =3D &(struct clk_init_data){
> > [...]
> >> +
> >> +static const struct regmap_config disp_cc_sdm845_regmap_config =3D {
> >> +       .reg_bits       =3D 32,
> >> +       .reg_stride     =3D 4,
> >> +       .val_bits       =3D 32,
> >> +       .max_register   =3D 0x10000,
> > =

> > This seems arbitrarily large. List the actual end?
> > =

> The actual end is larger than this :( , I have listed the range till the =

> register offset are used here.

Ok.

> =

> >> +       .fast_io        =3D true,
> >> +};
> >> +
> >> +static const struct qcom_cc_desc disp_cc_sdm845_desc =3D {
> >> +       .config =3D &disp_cc_sdm845_regmap_config,
> >> +       .clks =3D disp_cc_sdm845_clocks,
> >> +       .num_clks =3D ARRAY_SIZE(disp_cc_sdm845_clocks),
> >> +       .resets =3D disp_cc_sdm845_resets,
> >> +       .num_resets =3D ARRAY_SIZE(disp_cc_sdm845_resets),
> >> +       .gdscs =3D disp_cc_sdm845_gdscs,
> >> +       .num_gdscs =3D ARRAY_SIZE(disp_cc_sdm845_gdscs),
> >> +};
> >> +
> >> +static const struct of_device_id disp_cc_sdm845_match_table[] =3D {
> >> +       { .compatible =3D "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 =3D 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_c=
onfig);
> >> +
> >> +       /* Enable clock gating for DSI and MDP clocks */
> > =

> > Hardware clock gating? What does this mean.
> > =

> These are used for deciding whether to enable HW based Clock gating or =

> not. Setting these bit will enable HW based Clock gating.

Please state it's hardware clock gating then in the comment.

> =

> >> +       regmap_update_bits(regmap, 0x8000, 0x7f0, 0x7f0);
> >> +
> >> +       return qcom_cc_really_probe(pdev, &disp_cc_sdm845_desc, regmap=
);
> >> +}
>=20