devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
@ 2021-12-09 14:09 Loic Poulain
  2021-12-09 14:09 ` [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings Loic Poulain
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Loic Poulain @ 2021-12-09 14:09 UTC (permalink / raw)
  To: bjorn.andersson, agross, robh+dt
  Cc: linux-arm-msm, devicetree, linux-clk, shawn.guo, Loic Poulain

Add support for the display clock controller found in QCM2290
based devices. This clock controller feeds the Multimedia Display
SubSystem (MDSS).

It's a porting of dispcc-scuba GPL-2.0 driver from CAF msm-4.19 kernel:
https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/clk/qcom/dispcc-scuba.c?h=LE.UM.4.4.1.r3
Global clock name references (parent_names) have been replaced by
parent_data and parent_hws.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 v2: Added missing dispcc-qcm2290.h header file

 drivers/clk/qcom/Kconfig                        |   8 +
 drivers/clk/qcom/Makefile                       |   1 +
 drivers/clk/qcom/dispcc-qcm2290.c               | 602 ++++++++++++++++++++++++
 include/dt-bindings/clock/qcom,dispcc-qcm2290.h |  34 ++
 4 files changed, 645 insertions(+)
 create mode 100644 drivers/clk/qcom/dispcc-qcm2290.c
 create mode 100644 include/dt-bindings/clock/qcom,dispcc-qcm2290.h

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 74efc82..b136cd2 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -332,6 +332,14 @@ config QCM_GCC_2290
 	  Say Y if you want to use multimedia devices or peripheral
 	  devices such as UART, SPI, I2C, USB, SD/eMMC etc.
 
+config QCM_DISPCC_2290
+	tristate "QCM2290 Display Clock Controller"
+	help
+	  Support for the display clock controller on Qualcomm Technologies, Inc
+	  QCM2290 devices.
+	  Say Y if you want to support display devices and functionality such as
+	  splash screen.
+
 config QCS_GCC_404
 	tristate "QCS404 Global Clock Controller"
 	help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 1718c34..348d012 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
 obj-$(CONFIG_QCOM_CLK_RPMH) += clk-rpmh.o
 obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
 obj-$(CONFIG_QCM_GCC_2290) += gcc-qcm2290.o
+obj-$(CONFIG_QCM_DISPCC_2290) += dispcc-qcm2290.o
 obj-$(CONFIG_QCS_GCC_404) += gcc-qcs404.o
 obj-$(CONFIG_QCS_Q6SSTOP_404) += q6sstop-qcs404.o
 obj-$(CONFIG_QCS_TURING_404) += turingcc-qcs404.o
diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
new file mode 100644
index 00000000..8aa5d31
--- /dev/null
+++ b/drivers/clk/qcom/dispcc-qcm2290.c
@@ -0,0 +1,602 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2021, Linaro Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,dispcc-qcm2290.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "clk-regmap-divider.h"
+#include "common.h"
+#include "reset.h"
+#include "gdsc.h"
+
+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_DSICLK,
+	P_GPLL0_OUT_MAIN,
+	P_SLEEP_CLK,
+};
+
+static struct pll_vco spark_vco[] = {
+	{ 500000000, 1000000000, 2 },
+};
+
+/* 768MHz configuration */
+static const struct alpha_pll_config disp_cc_pll0_config = {
+	.l = 0x28,
+	.alpha = 0x0,
+	.alpha_en_mask = BIT(24),
+	.vco_val = 0x2 << 20,
+	.vco_mask = GENMASK(21, 20),
+	.main_output_mask = BIT(0),
+	.config_ctl_val = 0x4001055B,
+};
+
+static struct clk_alpha_pll disp_cc_pll0 = {
+	.offset = 0x0,
+	.vco_table = spark_vco,
+	.num_vco = ARRAY_SIZE(spark_vco),
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.clkr = {
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_pll0",
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "bi_tcxo",
+			},
+			.num_parents = 1,
+			.ops = &clk_alpha_pll_ops,
+		},
+	},
+};
+
+static const struct parent_map disp_cc_parent_map_0[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_DSI0_PHY_PLL_OUT_BYTECLK, 1 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_0[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "dsi0_phy_pll_out_byteclk" },
+	{ .fw_name = "core_bi_pll_test_se" },
+};
+
+static const struct parent_map disp_cc_parent_map_1[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_1[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "core_bi_pll_test_se" },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_1_ao[] = {
+	{ .fw_name = "bi_tcxo_ao" },
+	{ .fw_name = "core_bi_pll_test_se" },
+};
+
+static const struct parent_map disp_cc_parent_map_2[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_GPLL0_OUT_MAIN, 4 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_2[] = {
+	{ .fw_name = "bi_tcxo_ao" },
+	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },
+	{ .fw_name = "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_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_3[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .hw = &disp_cc_pll0.clkr.hw },
+	{ .fw_name = "gcc_disp_gpll0_clk_src" },
+	{ .fw_name = "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 struct clk_parent_data disp_cc_parent_data_4[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
+	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
+	{ .fw_name = "core_bi_pll_test_se" },
+};
+
+static const struct parent_map disp_cc_parent_map_5[] = {
+	{ P_SLEEP_CLK, 0 },
+	{ P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_5[] = {
+	{ .fw_name = "sleep_clk", .name = "sleep_clk" },
+	{ .fw_name = "core_bi_pll_test_se" },
+};
+
+static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = {
+	.cmd_rcgr = 0x20a4,
+	.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_data = disp_cc_parent_data_0,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
+		.ops = &clk_byte2_ops,
+	},
+};
+
+static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
+	.reg = 0x20bc,
+	.shift = 0,
+	.width = 2,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "disp_cc_mdss_byte0_div_clk_src",
+		.parent_hws = (const struct clk_hw*[]){
+			&disp_cc_mdss_byte0_clk_src.clkr.hw,
+		},
+		.num_parents = 1,
+		.flags = CLK_GET_RATE_NOCACHE,
+		.ops = &clk_regmap_div_ops,
+	},
+};
+
+static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
+	F(19200000, P_BI_TCXO, 1, 0, 0),
+	F(37500000, P_GPLL0_OUT_MAIN, 8, 0, 0),
+	F(75000000, P_GPLL0_OUT_MAIN, 4, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_ahb_clk_src = {
+	.cmd_rcgr = 0x2154,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_2,
+	.freq_tbl = ftbl_disp_cc_mdss_ahb_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_ahb_clk_src",
+		.parent_data = disp_cc_parent_data_2,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_2),
+		.ops = &clk_rcg2_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 = 0x20c0,
+	.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_data = disp_cc_parent_data_0,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
+		.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(192000000, P_DISP_CC_PLL0_OUT_MAIN, 4, 0, 0),
+	F(256000000, P_DISP_CC_PLL0_OUT_MAIN, 3, 0, 0),
+	F(307200000, P_DISP_CC_PLL0_OUT_MAIN, 2.5, 0, 0),
+	F(384000000, P_DISP_CC_PLL0_OUT_MAIN, 2, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
+	.cmd_rcgr = 0x2074,
+	.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_data = disp_cc_parent_data_3,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
+		.flags = CLK_SET_RATE_PARENT,
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
+	.cmd_rcgr = 0x205c,
+	.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_data = disp_cc_parent_data_4,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
+		.ops = &clk_pixel_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
+	.cmd_rcgr = 0x208c,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_1,
+	.freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_vsync_clk_src",
+		.parent_data = disp_cc_parent_data_1,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
+		.flags = CLK_SET_RATE_PARENT,
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static const struct freq_tbl ftbl_disp_cc_sleep_clk_src[] = {
+	F(32764, P_SLEEP_CLK, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_sleep_clk_src = {
+	.cmd_rcgr = 0x6050,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_5,
+	.freq_tbl = ftbl_disp_cc_sleep_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_sleep_clk_src",
+		.parent_data = disp_cc_parent_data_5,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_5),
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_xo_clk_src = {
+	.cmd_rcgr = 0x6034,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_1,
+	.freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_xo_clk_src",
+		.parent_data = disp_cc_parent_data_1_ao,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1_ao),
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static struct clk_branch disp_cc_mdss_ahb_clk = {
+	.halt_reg = 0x2044,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2044,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_ahb_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_ahb_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_byte0_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_byte0_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_byte0_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_byte0_intf_clk = {
+	.halt_reg = 0x2020,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2020,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_byte0_intf_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_byte0_div_clk_src.clkr.hw,
+			},
+			.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 = 0x2024,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2024,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_esc0_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_esc0_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_mdp_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_mdp_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_mdp_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_mdp_lut_clk = {
+	.halt_reg = 0x2010,
+	.halt_check = BRANCH_HALT_VOTED,
+	.clkr = {
+		.enable_reg = 0x2010,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_mdp_lut_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_mdp_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_non_gdsc_ahb_clk = {
+	.halt_reg = 0x4004,
+	.halt_check = BRANCH_HALT_VOTED,
+	.clkr = {
+		.enable_reg = 0x4004,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_non_gdsc_ahb_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_ahb_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.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_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_pclk0_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_vsync_clk = {
+	.halt_reg = 0x2018,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2018,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_vsync_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_vsync_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_sleep_clk = {
+	.halt_reg = 0x6068,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x6068,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_sleep_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_sleep_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_xo_clk = {
+	.halt_reg = 0x604c,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x604c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_xo_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_xo_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_IS_CRITICAL | 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,
+};
+
+static struct gdsc *disp_cc_qcm2290_gdscs[] = {
+	[MDSS_GDSC] = &mdss_gdsc,
+};
+
+static struct clk_regmap *disp_cc_qcm2290_clocks[] = {
+	[DISP_CC_MDSS_AHB_CLK] = &disp_cc_mdss_ahb_clk.clkr,
+	[DISP_CC_MDSS_AHB_CLK_SRC] = &disp_cc_mdss_ahb_clk_src.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_DIV_CLK_SRC] = &disp_cc_mdss_byte0_div_clk_src.clkr,
+	[DISP_CC_MDSS_BYTE0_INTF_CLK] = &disp_cc_mdss_byte0_intf_clk.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_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_NON_GDSC_AHB_CLK] = &disp_cc_mdss_non_gdsc_ahb_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_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,
+	[DISP_CC_SLEEP_CLK] = &disp_cc_sleep_clk.clkr,
+	[DISP_CC_SLEEP_CLK_SRC] = &disp_cc_sleep_clk_src.clkr,
+	[DISP_CC_XO_CLK] = &disp_cc_xo_clk.clkr,
+	[DISP_CC_XO_CLK_SRC] = &disp_cc_xo_clk_src.clkr,
+};
+
+static const struct regmap_config disp_cc_qcm2290_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_qcm2290_desc = {
+	.config = &disp_cc_qcm2290_regmap_config,
+	.clks = disp_cc_qcm2290_clocks,
+	.num_clks = ARRAY_SIZE(disp_cc_qcm2290_clocks),
+	.gdscs = disp_cc_qcm2290_gdscs,
+	.num_gdscs = ARRAY_SIZE(disp_cc_qcm2290_gdscs),
+};
+
+static const struct of_device_id disp_cc_qcm2290_match_table[] = {
+	{ .compatible = "qcom,qcm2290-dispcc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, disp_cc_qcm2290_match_table);
+
+static int disp_cc_qcm2290_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+	int ret;
+
+	regmap = qcom_cc_map(pdev, &disp_cc_qcm2290_desc);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	clk_alpha_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
+
+	ret = qcom_cc_really_probe(pdev, &disp_cc_qcm2290_desc, regmap);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register DISP CC clocks\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "Registered DISP CC clocks\n");
+
+	return ret;
+}
+
+static struct platform_driver disp_cc_qcm2290_driver = {
+	.probe = disp_cc_qcm2290_probe,
+	.driver = {
+		.name = "dispcc-qcm2290",
+		.of_match_table = disp_cc_qcm2290_match_table,
+	},
+};
+
+static int __init disp_cc_qcm2290_init(void)
+{
+	return platform_driver_register(&disp_cc_qcm2290_driver);
+}
+subsys_initcall(disp_cc_qcm2290_init);
+
+static void __exit disp_cc_qcm2290_exit(void)
+{
+	platform_driver_unregister(&disp_cc_qcm2290_driver);
+}
+module_exit(disp_cc_qcm2290_exit);
+
+MODULE_DESCRIPTION("QTI DISP_CC qcm2290 Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/clock/qcom,dispcc-qcm2290.h b/include/dt-bindings/clock/qcom,dispcc-qcm2290.h
new file mode 100644
index 00000000..ef50c17
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,dispcc-qcm2290.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2019, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_DISP_CC_SCUBA_H
+#define _DT_BINDINGS_CLK_QCOM_DISP_CC_SCUBA_H
+
+/* DISP_CC clocks */
+#define DISP_CC_PLL0				0
+#define DISP_CC_MDSS_AHB_CLK			1
+#define DISP_CC_MDSS_AHB_CLK_SRC		2
+#define DISP_CC_MDSS_BYTE0_CLK			3
+#define DISP_CC_MDSS_BYTE0_CLK_SRC		4
+#define DISP_CC_MDSS_BYTE0_DIV_CLK_SRC		5
+#define DISP_CC_MDSS_BYTE0_INTF_CLK		6
+#define DISP_CC_MDSS_ESC0_CLK			7
+#define DISP_CC_MDSS_ESC0_CLK_SRC		8
+#define DISP_CC_MDSS_MDP_CLK			9
+#define DISP_CC_MDSS_MDP_CLK_SRC		10
+#define DISP_CC_MDSS_MDP_LUT_CLK		11
+#define DISP_CC_MDSS_NON_GDSC_AHB_CLK		12
+#define DISP_CC_MDSS_PCLK0_CLK			13
+#define DISP_CC_MDSS_PCLK0_CLK_SRC		14
+#define DISP_CC_MDSS_VSYNC_CLK			15
+#define DISP_CC_MDSS_VSYNC_CLK_SRC		16
+#define DISP_CC_SLEEP_CLK			17
+#define DISP_CC_SLEEP_CLK_SRC			18
+#define DISP_CC_XO_CLK				19
+#define DISP_CC_XO_CLK_SRC			20
+
+#define MDSS_GDSC				0
+
+#endif
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings
  2021-12-09 14:09 [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290 Loic Poulain
@ 2021-12-09 14:09 ` Loic Poulain
  2021-12-15  3:41   ` Bjorn Andersson
  2021-12-15 18:13   ` Rob Herring
  2021-12-15  3:39 ` [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290 Bjorn Andersson
  2021-12-16  3:49 ` Stephen Boyd
  2 siblings, 2 replies; 11+ messages in thread
From: Loic Poulain @ 2021-12-09 14:09 UTC (permalink / raw)
  To: bjorn.andersson, agross, robh+dt
  Cc: linux-arm-msm, devicetree, linux-clk, shawn.guo, Loic Poulain

Add device tree bindings for display clock controller on QCM2290 SoCs.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 v2: no change

 .../bindings/clock/qcom,qcm2290-dispcc.yaml        | 87 ++++++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml

diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
new file mode 100644
index 00000000..44d5ce7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,qcm2290-dispcc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Display Clock & Reset Controller Binding for qcm2290
+
+maintainers:
+  - Loic Poulain <loic.poulain@linaro.org>
+
+description: |
+  Qualcomm display clock control module which supports the clocks, resets and
+  power domains on qcm2290.
+
+  See also dt-bindings/clock/qcom,dispcc-qcm2290.h.
+
+properties:
+  compatible:
+    const: qcom,qcm2290-dispcc
+
+  clocks:
+    items:
+      - description: Board XO source
+      - description: Board active-only XO source
+      - description: GPLL0 source from GCC
+      - description: GPLL0 div source from GCC
+      - description: Byte clock from DSI PHY
+      - description: Pixel clock from DSI PHY
+
+  clock-names:
+    items:
+      - const: bi_tcxo
+      - const: bi_tcxo_ao
+      - const: gcc_disp_gpll0_clk_src
+      - const: gcc_disp_gpll0_div_clk_src
+      - const: dsi0_phy_pll_out_byteclk
+      - const: dsi0_phy_pll_out_dsiclk
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#reset-cells'
+  - '#power-domain-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,dispcc-qcm2290.h>
+    #include <dt-bindings/clock/qcom,gcc-qcm2290.h>
+    #include <dt-bindings/clock/qcom,rpmcc.h>
+    clock-controller@5f00000 {
+            compatible = "qcom,qcm2290-dispcc";
+            reg = <0x5f00000 0x20000>;
+            clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+                     <&rpmcc RPM_SMD_XO_A_CLK_SRC>,
+                     <&gcc GCC_DISP_GPLL0_CLK_SRC>,
+                     <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
+                     <&dsi0_phy 0>,
+                     <&dsi0_phy 1>;
+            clock-names = "bi_tcxo",
+                          "bi_tcxo_ao",
+                          "gcc_disp_gpll0_clk_src",
+                          "gcc_disp_gpll0_div_clk_src",
+                          "dsi0_phy_pll_out_byteclk",
+                          "dsi0_phy_pll_out_dsiclk";
+            #clock-cells = <1>;
+            #reset-cells = <1>;
+            #power-domain-cells = <1>;
+    };
+...
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
  2021-12-09 14:09 [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290 Loic Poulain
  2021-12-09 14:09 ` [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings Loic Poulain
@ 2021-12-15  3:39 ` Bjorn Andersson
  2021-12-16  3:49 ` Stephen Boyd
  2 siblings, 0 replies; 11+ messages in thread
From: Bjorn Andersson @ 2021-12-15  3:39 UTC (permalink / raw)
  To: Loic Poulain
  Cc: agross, robh+dt, linux-arm-msm, devicetree, linux-clk, shawn.guo

On Thu 09 Dec 08:09 CST 2021, Loic Poulain wrote:
[..]
> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
[..]
> +static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
> +	.cmd_rcgr = 0x2074,
> +	.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_data = disp_cc_parent_data_3,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> +		.flags = CLK_SET_RATE_PARENT,
> +		.ops = &clk_rcg2_ops,

Clocks that are marked enable_safe_config = true downstream needs to be
parked onto a safe parent as they are being disabled.

This is taken care of upstream by using clk_rcg2_shared_ops.

> +	},
> +};
> +
[..]
> diff --git a/include/dt-bindings/clock/qcom,dispcc-qcm2290.h b/include/dt-bindings/clock/qcom,dispcc-qcm2290.h
> new file mode 100644
> index 00000000..ef50c17
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,dispcc-qcm2290.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_DISP_CC_SCUBA_H
> +#define _DT_BINDINGS_CLK_QCOM_DISP_CC_SCUBA_H

Please replace SCUBA here and can you please move the include file to
the DT binding patch instead?

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings
  2021-12-09 14:09 ` [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings Loic Poulain
@ 2021-12-15  3:41   ` Bjorn Andersson
  2021-12-15 18:13   ` Rob Herring
  1 sibling, 0 replies; 11+ messages in thread
From: Bjorn Andersson @ 2021-12-15  3:41 UTC (permalink / raw)
  To: Loic Poulain
  Cc: agross, robh+dt, linux-arm-msm, devicetree, linux-clk, shawn.guo

On Thu 09 Dec 08:09 CST 2021, Loic Poulain wrote:

> Add device tree bindings for display clock controller on QCM2290 SoCs.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  v2: no change
> 
>  .../bindings/clock/qcom,qcm2290-dispcc.yaml        | 87 ++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> new file mode 100644
> index 00000000..44d5ce7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,qcm2290-dispcc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Display Clock & Reset Controller Binding for qcm2290
> +
> +maintainers:
> +  - Loic Poulain <loic.poulain@linaro.org>
> +
> +description: |
> +  Qualcomm display clock control module which supports the clocks, resets and
> +  power domains on qcm2290.
> +
> +  See also dt-bindings/clock/qcom,dispcc-qcm2290.h.
> +
> +properties:
> +  compatible:
> +    const: qcom,qcm2290-dispcc
> +
> +  clocks:
> +    items:
> +      - description: Board XO source
> +      - description: Board active-only XO source
> +      - description: GPLL0 source from GCC
> +      - description: GPLL0 div source from GCC
> +      - description: Byte clock from DSI PHY
> +      - description: Pixel clock from DSI PHY
> +
> +  clock-names:
> +    items:
> +      - const: bi_tcxo
> +      - const: bi_tcxo_ao
> +      - const: gcc_disp_gpll0_clk_src
> +      - const: gcc_disp_gpll0_div_clk_src
> +      - const: dsi0_phy_pll_out_byteclk
> +      - const: dsi0_phy_pll_out_dsiclk
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#reset-cells':
> +    const: 1
> +
> +  '#power-domain-cells':
> +    const: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +  - '#power-domain-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,dispcc-qcm2290.h>
> +    #include <dt-bindings/clock/qcom,gcc-qcm2290.h>
> +    #include <dt-bindings/clock/qcom,rpmcc.h>
> +    clock-controller@5f00000 {
> +            compatible = "qcom,qcm2290-dispcc";
> +            reg = <0x5f00000 0x20000>;
> +            clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +                     <&rpmcc RPM_SMD_XO_A_CLK_SRC>,
> +                     <&gcc GCC_DISP_GPLL0_CLK_SRC>,
> +                     <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
> +                     <&dsi0_phy 0>,
> +                     <&dsi0_phy 1>;
> +            clock-names = "bi_tcxo",
> +                          "bi_tcxo_ao",
> +                          "gcc_disp_gpll0_clk_src",
> +                          "gcc_disp_gpll0_div_clk_src",
> +                          "dsi0_phy_pll_out_byteclk",
> +                          "dsi0_phy_pll_out_dsiclk";
> +            #clock-cells = <1>;
> +            #reset-cells = <1>;
> +            #power-domain-cells = <1>;
> +    };
> +...
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings
  2021-12-09 14:09 ` [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings Loic Poulain
  2021-12-15  3:41   ` Bjorn Andersson
@ 2021-12-15 18:13   ` Rob Herring
  1 sibling, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-12-15 18:13 UTC (permalink / raw)
  To: Loic Poulain
  Cc: bjorn.andersson, agross, linux-arm-msm, devicetree, linux-clk, shawn.guo

On Thu, Dec 09, 2021 at 03:09:11PM +0100, Loic Poulain wrote:
> Add device tree bindings for display clock controller on QCM2290 SoCs.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  v2: no change
> 
>  .../bindings/clock/qcom,qcm2290-dispcc.yaml        | 87 ++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> new file mode 100644
> index 00000000..44d5ce7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: GPL-2.0-only

This and the header should be dual licensed. Not necessarily the same 
ones. The header should match Qcom dts files.

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
  2021-12-09 14:09 [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290 Loic Poulain
  2021-12-09 14:09 ` [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings Loic Poulain
  2021-12-15  3:39 ` [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290 Bjorn Andersson
@ 2021-12-16  3:49 ` Stephen Boyd
  2021-12-16 19:21   ` Loic Poulain
  2 siblings, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2021-12-16  3:49 UTC (permalink / raw)
  To: Loic Poulain, agross, bjorn.andersson, robh+dt
  Cc: linux-arm-msm, devicetree, linux-clk, shawn.guo, Loic Poulain

Quoting Loic Poulain (2021-12-09 06:09:10)
> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> new file mode 100644
> index 00000000..8aa5d31
> --- /dev/null
> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> @@ -0,0 +1,602 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2021, Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>

Is this include used?

> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,dispcc-qcm2290.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "clk-regmap-divider.h"
> +#include "common.h"
> +#include "reset.h"
> +#include "gdsc.h"
> +
> +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_DSICLK,
> +       P_GPLL0_OUT_MAIN,
> +       P_SLEEP_CLK,
> +};
> +
> +static struct pll_vco spark_vco[] = {

const

> +       { 500000000, 1000000000, 2 },
> +};
> +
> +/* 768MHz configuration */
> +static const struct alpha_pll_config disp_cc_pll0_config = {
> +       .l = 0x28,
> +       .alpha = 0x0,
> +       .alpha_en_mask = BIT(24),
> +       .vco_val = 0x2 << 20,
> +       .vco_mask = GENMASK(21, 20),
> +       .main_output_mask = BIT(0),
> +       .config_ctl_val = 0x4001055B,
> +};
> +
> +static struct clk_alpha_pll disp_cc_pll0 = {
> +       .offset = 0x0,
> +       .vco_table = spark_vco,
> +       .num_vco = ARRAY_SIZE(spark_vco),
> +       .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> +       .clkr = {
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_pll0",
> +                       .parent_data = &(const struct clk_parent_data){
> +                               .fw_name = "bi_tcxo",
> +                       },
> +                       .num_parents = 1,
> +                       .ops = &clk_alpha_pll_ops,
> +               },
> +       },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_0[] = {
> +       { P_BI_TCXO, 0 },
> +       { P_DSI0_PHY_PLL_OUT_BYTECLK, 1 },
> +       { P_CORE_BI_PLL_TEST_SE, 7 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_0[] = {
> +       { .fw_name = "bi_tcxo" },
> +       { .fw_name = "dsi0_phy_pll_out_byteclk" },
> +       { .fw_name = "core_bi_pll_test_se" },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_1[] = {
> +       { P_BI_TCXO, 0 },
> +       { P_CORE_BI_PLL_TEST_SE, 7 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_1[] = {
> +       { .fw_name = "bi_tcxo" },
> +       { .fw_name = "core_bi_pll_test_se" },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_1_ao[] = {
> +       { .fw_name = "bi_tcxo_ao" },
> +       { .fw_name = "core_bi_pll_test_se" },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_2[] = {
> +       { P_BI_TCXO, 0 },
> +       { P_GPLL0_OUT_MAIN, 4 },
> +       { P_CORE_BI_PLL_TEST_SE, 7 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_2[] = {
> +       { .fw_name = "bi_tcxo_ao" },
> +       { .fw_name = "gcc_disp_gpll0_div_clk_src" },
> +       { .fw_name = "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_CORE_BI_PLL_TEST_SE, 7 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_3[] = {
> +       { .fw_name = "bi_tcxo" },
> +       { .hw = &disp_cc_pll0.clkr.hw },
> +       { .fw_name = "gcc_disp_gpll0_clk_src" },
> +       { .fw_name = "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 struct clk_parent_data disp_cc_parent_data_4[] = {
> +       { .fw_name = "bi_tcxo" },
> +       { .fw_name = "dsi0_phy_pll_out_dsiclk" },
> +       { .fw_name = "dsi1_phy_pll_out_dsiclk" },
> +       { .fw_name = "core_bi_pll_test_se" },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_5[] = {
> +       { P_SLEEP_CLK, 0 },
> +       { P_CORE_BI_PLL_TEST_SE, 7 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_5[] = {
> +       { .fw_name = "sleep_clk", .name = "sleep_clk" },

Please don't add .name to clk_parent_data if this is a new driver.

> +       { .fw_name = "core_bi_pll_test_se" },
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = {
> +       .cmd_rcgr = 0x20a4,
> +       .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_data = disp_cc_parent_data_0,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
> +               .ops = &clk_byte2_ops,
> +       },
> +};
> +
> +static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
> +       .reg = 0x20bc,
> +       .shift = 0,
> +       .width = 2,
> +       .clkr.hw.init = &(struct clk_init_data) {
> +               .name = "disp_cc_mdss_byte0_div_clk_src",
> +               .parent_hws = (const struct clk_hw*[]){
> +                       &disp_cc_mdss_byte0_clk_src.clkr.hw,
> +               },
> +               .num_parents = 1,
> +               .flags = CLK_GET_RATE_NOCACHE,
> +               .ops = &clk_regmap_div_ops,
> +       },
> +};
> +
> +static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
> +       F(19200000, P_BI_TCXO, 1, 0, 0),
> +       F(37500000, P_GPLL0_OUT_MAIN, 8, 0, 0),
> +       F(75000000, P_GPLL0_OUT_MAIN, 4, 0, 0),
> +       { }
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_ahb_clk_src = {
> +       .cmd_rcgr = 0x2154,
> +       .mnd_width = 0,
> +       .hid_width = 5,
> +       .parent_map = disp_cc_parent_map_2,
> +       .freq_tbl = ftbl_disp_cc_mdss_ahb_clk_src,
> +       .clkr.hw.init = &(struct clk_init_data){
> +               .name = "disp_cc_mdss_ahb_clk_src",
> +               .parent_data = disp_cc_parent_data_2,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_2),
> +               .ops = &clk_rcg2_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 = 0x20c0,
> +       .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_data = disp_cc_parent_data_0,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> +               .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(192000000, P_DISP_CC_PLL0_OUT_MAIN, 4, 0, 0),
> +       F(256000000, P_DISP_CC_PLL0_OUT_MAIN, 3, 0, 0),
> +       F(307200000, P_DISP_CC_PLL0_OUT_MAIN, 2.5, 0, 0),
> +       F(384000000, P_DISP_CC_PLL0_OUT_MAIN, 2, 0, 0),
> +       { }
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
> +       .cmd_rcgr = 0x2074,
> +       .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_data = disp_cc_parent_data_3,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> +               .flags = CLK_SET_RATE_PARENT,
> +               .ops = &clk_rcg2_ops,
> +       },
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
> +       .cmd_rcgr = 0x205c,
> +       .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_data = disp_cc_parent_data_4,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
> +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,

These last two flags are needed for what?

> +               .ops = &clk_pixel_ops,
> +       },
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
> +       .cmd_rcgr = 0x208c,
> +       .mnd_width = 0,
> +       .hid_width = 5,
> +       .parent_map = disp_cc_parent_map_1,
> +       .freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
> +       .clkr.hw.init = &(struct clk_init_data){
> +               .name = "disp_cc_mdss_vsync_clk_src",
> +               .parent_data = disp_cc_parent_data_1,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
> +               .flags = CLK_SET_RATE_PARENT,
> +               .ops = &clk_rcg2_ops,
> +       },
> +};
> +
> +static const struct freq_tbl ftbl_disp_cc_sleep_clk_src[] = {
> +       F(32764, P_SLEEP_CLK, 1, 0, 0),
> +       { }
> +};
> +
> +static struct clk_rcg2 disp_cc_sleep_clk_src = {
> +       .cmd_rcgr = 0x6050,
> +       .mnd_width = 0,
> +       .hid_width = 5,
> +       .parent_map = disp_cc_parent_map_5,
> +       .freq_tbl = ftbl_disp_cc_sleep_clk_src,
> +       .clkr.hw.init = &(struct clk_init_data){
> +               .name = "disp_cc_sleep_clk_src",
> +               .parent_data = disp_cc_parent_data_5,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_5),
> +               .ops = &clk_rcg2_ops,
> +       },
> +};
> +
> +static struct clk_rcg2 disp_cc_xo_clk_src = {
> +       .cmd_rcgr = 0x6034,
> +       .mnd_width = 0,
> +       .hid_width = 5,
> +       .parent_map = disp_cc_parent_map_1,
> +       .freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
> +       .clkr.hw.init = &(struct clk_init_data){
> +               .name = "disp_cc_xo_clk_src",
> +               .parent_data = disp_cc_parent_data_1_ao,
> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_1_ao),
> +               .ops = &clk_rcg2_ops,
> +       },
> +};
> +
> +static struct clk_branch disp_cc_mdss_ahb_clk = {
> +       .halt_reg = 0x2044,
> +       .halt_check = BRANCH_HALT,
> +       .clkr = {
> +               .enable_reg = 0x2044,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_mdss_ahb_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_ahb_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_mdss_byte0_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_byte0_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_byte0_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_mdss_byte0_intf_clk = {
> +       .halt_reg = 0x2020,
> +       .halt_check = BRANCH_HALT,
> +       .clkr = {
> +               .enable_reg = 0x2020,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_mdss_byte0_intf_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_byte0_div_clk_src.clkr.hw,
> +                       },
> +                       .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 = 0x2024,
> +       .halt_check = BRANCH_HALT,
> +       .clkr = {
> +               .enable_reg = 0x2024,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_mdss_esc0_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_esc0_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_mdss_mdp_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_mdp_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_mdp_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_mdss_mdp_lut_clk = {
> +       .halt_reg = 0x2010,
> +       .halt_check = BRANCH_HALT_VOTED,
> +       .clkr = {
> +               .enable_reg = 0x2010,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_mdss_mdp_lut_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_mdp_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_mdss_non_gdsc_ahb_clk = {
> +       .halt_reg = 0x4004,
> +       .halt_check = BRANCH_HALT_VOTED,
> +       .clkr = {
> +               .enable_reg = 0x4004,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_mdss_non_gdsc_ahb_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_ahb_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT,
> +                       .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_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_pclk0_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_mdss_vsync_clk = {
> +       .halt_reg = 0x2018,
> +       .halt_check = BRANCH_HALT,
> +       .clkr = {
> +               .enable_reg = 0x2018,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_mdss_vsync_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_mdss_vsync_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_sleep_clk = {
> +       .halt_reg = 0x6068,
> +       .halt_check = BRANCH_HALT,
> +       .clkr = {
> +               .enable_reg = 0x6068,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_sleep_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_sleep_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_SET_RATE_PARENT,
> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +
> +static struct clk_branch disp_cc_xo_clk = {
> +       .halt_reg = 0x604c,
> +       .halt_check = BRANCH_HALT,
> +       .clkr = {
> +               .enable_reg = 0x604c,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "disp_cc_xo_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &disp_cc_xo_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,

We need a comment why it's critical. Also I'm not sure why we would ever
turn this clk off or change the rate. Can't we just hit some registers
during probe to make sure it's on and drop this clk?

> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
  2021-12-16  3:49 ` Stephen Boyd
@ 2021-12-16 19:21   ` Loic Poulain
  2021-12-17  1:37     ` Stephen Boyd
  2021-12-17 16:16     ` Steev Klimaszewski
  0 siblings, 2 replies; 11+ messages in thread
From: Loic Poulain @ 2021-12-16 19:21 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: agross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-clk, shawn.guo

Hi Stephen,


On Thu, 16 Dec 2021 at 04:49, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Loic Poulain (2021-12-09 06:09:10)
> > diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> > new file mode 100644
> > index 00000000..8aa5d31
> > --- /dev/null
> > +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> > @@ -0,0 +1,602 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> > + * Copyright (c) 2021, Linaro Ltd.
> > + */
> > +
[...]
> > +static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
> > +       .cmd_rcgr = 0x205c,
> > +       .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_data = disp_cc_parent_data_4,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
> > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
>
> These last two flags are needed for what?

NOCACHE is probably useless with mainline.

I've added OPS_PARENT_ENABLE because AFAIU changing clock rate can
lead to parent switch, and parent switch can only be done if parent
clocks are enabled for rcg2 clocks. Otherwise the update fails and we
get the following:
    disp_cc_mdss_pclk0_clk_src: rcg didn't update its configuration.
    WARNING: CPU: 2 PID: 77 at drivers/clk/qcom/clk-rcg2.c:122
update_config+0xe0/0xf0

I'm a bit surprised other similar dispcc drivers don't use the same
flags though.


>
> > +               .ops = &clk_pixel_ops,
> > +       },
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
> > +       .cmd_rcgr = 0x208c,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_1,
> > +       .freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_vsync_clk_src",
> > +               .parent_data = disp_cc_parent_data_1,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
> > +               .flags = CLK_SET_RATE_PARENT,
> > +               .ops = &clk_rcg2_ops,
> > +       },
> > +};
> > +
[...]
> > +
> > +static struct clk_branch disp_cc_xo_clk = {
> > +       .halt_reg = 0x604c,
> > +       .halt_check = BRANCH_HALT,
> > +       .clkr = {
> > +               .enable_reg = 0x604c,
> > +               .enable_mask = BIT(0),
> > +               .hw.init = &(struct clk_init_data){
> > +                       .name = "disp_cc_xo_clk",
> > +                       .parent_hws = (const struct clk_hw*[]){
> > +                               &disp_cc_xo_clk_src.clkr.hw,
> > +                       },
> > +                       .num_parents = 1,
> > +                       .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
>
> We need a comment why it's critical. Also I'm not sure why we would ever
> turn this clk off or change the rate. Can't we just hit some registers
> during probe to make sure it's on and drop this clk?

Yes, good point, no objection, we will lose the hierarchical clk view
for this clk (up to bi_tcxo_ao), but it does not really matter.

Regards,
Loic

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
  2021-12-16 19:21   ` Loic Poulain
@ 2021-12-17  1:37     ` Stephen Boyd
  2021-12-17  1:58       ` Dmitry Baryshkov
  2021-12-17 16:16     ` Steev Klimaszewski
  1 sibling, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2021-12-17  1:37 UTC (permalink / raw)
  To: Loic Poulain
  Cc: agross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-clk, shawn.guo

Quoting Loic Poulain (2021-12-16 11:21:51)
> Hi Stephen,
> 
> 
> On Thu, 16 Dec 2021 at 04:49, Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Loic Poulain (2021-12-09 06:09:10)
> > > diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> > > new file mode 100644
> > > index 00000000..8aa5d31
> > > --- /dev/null
> > > +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> > > @@ -0,0 +1,602 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> > > + * Copyright (c) 2021, Linaro Ltd.
> > > + */
> > > +
> [...]
> > > +static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
> > > +       .cmd_rcgr = 0x205c,
> > > +       .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_data = disp_cc_parent_data_4,
> > > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
> > > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
> >
> > These last two flags are needed for what?
> 
> NOCACHE is probably useless with mainline.

Ok then please remove it.

> 
> I've added OPS_PARENT_ENABLE because AFAIU changing clock rate can
> lead to parent switch, and parent switch can only be done if parent
> clocks are enabled for rcg2 clocks. Otherwise the update fails and we
> get the following:
>     disp_cc_mdss_pclk0_clk_src: rcg didn't update its configuration.
>     WARNING: CPU: 2 PID: 77 at drivers/clk/qcom/clk-rcg2.c:122
> update_config+0xe0/0xf0
> 
> I'm a bit surprised other similar dispcc drivers don't use the same
> flags though.

That's quite odd. We migrate the prepare and enable count to the new
parent in the core framework so is the rcg on, but doesn't look like it
is on to the core and set_rate is being called?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
  2021-12-17  1:37     ` Stephen Boyd
@ 2021-12-17  1:58       ` Dmitry Baryshkov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-12-17  1:58 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Loic Poulain, agross, bjorn.andersson, robh+dt, linux-arm-msm,
	devicetree, linux-clk, shawn.guo

On Fri, 17 Dec 2021 at 04:37, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Loic Poulain (2021-12-16 11:21:51)
> > Hi Stephen,
> >
> >
> > On Thu, 16 Dec 2021 at 04:49, Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Loic Poulain (2021-12-09 06:09:10)
> > > > diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> > > > new file mode 100644
> > > > index 00000000..8aa5d31
> > > > --- /dev/null
> > > > +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> > > > @@ -0,0 +1,602 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> > > > + * Copyright (c) 2021, Linaro Ltd.
> > > > + */
> > > > +
> > [...]
> > > > +static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
> > > > +       .cmd_rcgr = 0x205c,
> > > > +       .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_data = disp_cc_parent_data_4,
> > > > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
> > > > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
> > >
> > > These last two flags are needed for what?
> >
> > NOCACHE is probably useless with mainline.
>
> Ok then please remove it.
>
> >
> > I've added OPS_PARENT_ENABLE because AFAIU changing clock rate can
> > lead to parent switch, and parent switch can only be done if parent
> > clocks are enabled for rcg2 clocks. Otherwise the update fails and we
> > get the following:
> >     disp_cc_mdss_pclk0_clk_src: rcg didn't update its configuration.
> >     WARNING: CPU: 2 PID: 77 at drivers/clk/qcom/clk-rcg2.c:122
> > update_config+0xe0/0xf0
> >
> > I'm a bit surprised other similar dispcc drivers don't use the same
> > flags though.
>
> That's quite odd. We migrate the prepare and enable count to the new
> parent in the core framework so is the rcg on, but doesn't look like it
> is on to the core and set_rate is being called?

It's a part of a typical problem for some clocks (that we were
discussing in a thread regarding Bjorn's ASSUME_ENABLED and my 'park
clocks' proposal). For set_rate and set_parent to succeed, both old
and new parrents must be running at the time of the operation/



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
  2021-12-16 19:21   ` Loic Poulain
  2021-12-17  1:37     ` Stephen Boyd
@ 2021-12-17 16:16     ` Steev Klimaszewski
  2021-12-17 16:54       ` Loic Poulain
  1 sibling, 1 reply; 11+ messages in thread
From: Steev Klimaszewski @ 2021-12-17 16:16 UTC (permalink / raw)
  To: Loic Poulain, Stephen Boyd
  Cc: agross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-clk, shawn.guo

Hi Loic,


>>> +       .cmd_rcgr = 0x205c,
>>> +       .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_data = disp_cc_parent_data_4,
>>> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
>>> +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
>> These last two flags are needed for what?
> NOCACHE is probably useless with mainline.
>
> I've added OPS_PARENT_ENABLE because AFAIU changing clock rate can
> lead to parent switch, and parent switch can only be done if parent
> clocks are enabled for rcg2 clocks. Otherwise the update fails and we
> get the following:
>      disp_cc_mdss_pclk0_clk_src: rcg didn't update its configuration.
>      WARNING: CPU: 2 PID: 77 at drivers/clk/qcom/clk-rcg2.c:122
> update_config+0xe0/0xf0
>
> I'm a bit surprised other similar dispcc drivers don't use the same
> flags though.
>
So, we do actually see this on other devices - in particular, on the 
Lenovo Yoga C630, and people have been trying to track down the initial 
cause for a while.  I tried here adding CLK_OPS_PARENT_ENABLE to both 
disp_cc_mdss_mdp_clk and disp_cc_mdss_pclk0_clk in dispcc-sdm845.c as 
well as for video_cc_venus_clk_src in videocc-sdm845.c and while it does 
seem to cause the messages to go away for disp_cc_mdss_mdp_clk and 
disp_cc_mdss_pclk0_clk, the one for venus seems to continue to show up here.

video_cc_venus_clk_src: rcg didn't update its configuration. WARNING: 
CPU: 1 PID: 404 at drivers/clk/qcom/clk-rcg2.c:122 update_config+0xd0/0xe0

I'm not sure if this is due to venus being a module and not built-in.

-- steev


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290
  2021-12-17 16:16     ` Steev Klimaszewski
@ 2021-12-17 16:54       ` Loic Poulain
  0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2021-12-17 16:54 UTC (permalink / raw)
  To: Steev Klimaszewski
  Cc: Stephen Boyd, agross, bjorn.andersson, robh+dt, linux-arm-msm,
	devicetree, linux-clk, shawn.guo, Dmitry Baryshkov

Hi Steev,

On Fri, 17 Dec 2021 at 17:16, Steev Klimaszewski <steev@kali.org> wrote:
> >>> +       .cmd_rcgr = 0x205c,
> >>> +       .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_data = disp_cc_parent_data_4,
> >>> +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
> >>> +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
> >> These last two flags are needed for what?
> > NOCACHE is probably useless with mainline.
> >
> > I've added OPS_PARENT_ENABLE because AFAIU changing clock rate can
> > lead to parent switch, and parent switch can only be done if parent
> > clocks are enabled for rcg2 clocks. Otherwise the update fails and we
> > get the following:
> >      disp_cc_mdss_pclk0_clk_src: rcg didn't update its configuration.
> >      WARNING: CPU: 2 PID: 77 at drivers/clk/qcom/clk-rcg2.c:122
> > update_config+0xe0/0xf0
> >
> > I'm a bit surprised other similar dispcc drivers don't use the same
> > flags though.
> >
> So, we do actually see this on other devices - in particular, on the
> Lenovo Yoga C630, and people have been trying to track down the initial
> cause for a while.  I tried here adding CLK_OPS_PARENT_ENABLE to both
> disp_cc_mdss_mdp_clk and disp_cc_mdss_pclk0_clk in dispcc-sdm845.c as
> well as for video_cc_venus_clk_src in videocc-sdm845.c and while it does
> seem to cause the messages to go away for disp_cc_mdss_mdp_clk and
> disp_cc_mdss_pclk0_clk, the one for venus seems to continue to show up here.
>
> video_cc_venus_clk_src: rcg didn't update its configuration. WARNING:
> CPU: 1 PID: 404 at drivers/clk/qcom/clk-rcg2.c:122 update_config+0xd0/0xe0
>
> I'm not sure if this is due to venus being a module and not built-in.

I think what you should do is dumping clk configuration before update
(basically for clk_rcg2_set_parent, __clk_rcg2_configure), especially
which parent source is configured for this clock. As Dmitry said, For
set_rate and set_parent to succeed, both old and new parents must be
running at the time of the operation. But in videocc-sdm845.cc I see
that some parents for video_cc_venus_clk_src are commented, would it
be possible that video_cc_venus_clk_src is configured with one of
these unhandled parents at boot time, making parent switch impossible?

Regards,
Loic

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-12-17 16:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 14:09 [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290 Loic Poulain
2021-12-09 14:09 ` [PATCH v2 2/2] dt-bindings: clock: Add qualcomm QCM2290 DISPCC bindings Loic Poulain
2021-12-15  3:41   ` Bjorn Andersson
2021-12-15 18:13   ` Rob Herring
2021-12-15  3:39 ` [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290 Bjorn Andersson
2021-12-16  3:49 ` Stephen Boyd
2021-12-16 19:21   ` Loic Poulain
2021-12-17  1:37     ` Stephen Boyd
2021-12-17  1:58       ` Dmitry Baryshkov
2021-12-17 16:16     ` Steev Klimaszewski
2021-12-17 16:54       ` Loic Poulain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).