[PATCH v2 0/3] clk: qcom: Add display clock controller driver for SM6125

[PATCH v2 0/3] clk: qcom: Add display clock controller driver for SM6125

[Marijn Suijten]

Changes since v1:
- Documentation is dual-licensed;
- Documentation example now uses zero-clock for dsi1_phy pixel clock;
- SDX_GCC_65 is sorted in Kconfig/Makefile to easen adding this driver
  in the correct alphabetic spot;
- clk.h is replaced with clk-provider.h;
- ahb, mdp and rot source clocks use rcg2_shared_ops instead of standard
  ops;
- Unnecessary line breaks are removed when remaining under 80 chars.

v1: https://lore.kernel.org/linux-arm-msm/20211130212137.25303-1-martin.botka@somainline.org/T/#u

Marijn Suijten (1):
  clk: qcom: Fix sorting of SDX_GCC_65 in Makefile and Kconfig

Martin Botka (2):
  dt-bindings: clock: add QCOM SM6125 display clock bindings
  clk: qcom: Add display clock controller driver for SM6125

 .../bindings/clock/qcom,dispcc-sm6125.yaml    |  87 +++
 drivers/clk/qcom/Kconfig                      |  21 +-
 drivers/clk/qcom/Makefile                     |   3 +-
 drivers/clk/qcom/dispcc-sm6125.c              | 709 ++++++++++++++++++
 .../dt-bindings/clock/qcom,dispcc-sm6125.h    |  41 +
 5 files changed, 854 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
 create mode 100644 drivers/clk/qcom/dispcc-sm6125.c
 create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h


base-commit: 06aeb1495c39c86ccfaf1adadc1d2200179f16eb
--
2.35.1

[PATCH v2 1/3] clk: qcom: Fix sorting of SDX_GCC_65 in Makefile and Kconfig

[Marijn Suijten]

In order to keep at least the list of `CONFIG_SM_` drivers sorted
alphabetically, SDX_GCC_65 should have been moved one line up.  This in
turn makes it easier and cleaner to add the followup SM_DISPCC_6125
driver in the right place, right before SM_DISPCC_8250.

Fixes: d79afa201328 ("clk: qcom: Add SDX65 GCC support")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/clk/qcom/Kconfig  | 14 +++++++-------
 drivers/clk/qcom/Makefile |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index f5b54bfc992f..161b257da9ca 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -574,13 +574,6 @@ config SDX_GCC_55
 	  Say Y if you want to use peripheral devices such as UART,
 	  SPI, I2C, USB, SD/UFS, PCIe etc.
 
-config SM_CAMCC_8250
-	tristate "SM8250 Camera Clock Controller"
-	select SM_GCC_8250
-	help
-	  Support for the camera clock controller on SM8250 devices.
-	  Say Y if you want to support camera devices and camera functionality.
-
 config SDX_GCC_65
 	tristate "SDX65 Global Clock Controller"
 	select QCOM_GDSC
@@ -589,6 +582,13 @@ config SDX_GCC_65
 	  Say Y if you want to use peripheral devices such as UART,
 	  SPI, I2C, USB, SD/UFS, PCIe etc.
 
+config SM_CAMCC_8250
+	tristate "SM8250 Camera Clock Controller"
+	select SM_GCC_8250
+	help
+	  Support for the camera clock controller on SM8250 devices.
+	  Say Y if you want to support camera devices and camera functionality.
+
 config SM_DISPCC_8250
 	tristate "SM8150 and SM8250 Display Clock Controller"
 	depends on SM_GCC_8150 || SM_GCC_8250
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index d96d6793fc7d..3e4eb843b8d2 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -84,8 +84,8 @@ obj-$(CONFIG_SDM_GPUCC_845) += gpucc-sdm845.o
 obj-$(CONFIG_SDM_LPASSCC_845) += lpasscc-sdm845.o
 obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o
 obj-$(CONFIG_SDX_GCC_55) += gcc-sdx55.o
-obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o
 obj-$(CONFIG_SDX_GCC_65) += gcc-sdx65.o
+obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o
 obj-$(CONFIG_SM_DISPCC_8250) += dispcc-sm8250.o
 obj-$(CONFIG_SM_GCC_6115) += gcc-sm6115.o
 obj-$(CONFIG_SM_GCC_6125) += gcc-sm6125.o
-- 
2.35.1

[PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Marijn Suijten]

From: Martin Botka <martin.botka@somainline.org>

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

Signed-off-by: Martin Botka <martin.botka@somainline.org>
---
 .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
 .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
 2 files changed, 128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
 create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h

diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
new file mode 100644
index 000000000000..3465042d0d9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Display Clock Controller Binding for SM6125
+
+maintainers:
+  - Martin Botka <martin.botka@somainline.org>
+
+description: |
+  Qualcomm display clock control module which supports the clocks and
+  power domains on SM6125.
+
+  See also:
+    dt-bindings/clock/qcom,dispcc-sm6125.h
+
+properties:
+  compatible:
+    enum:
+      - qcom,sm6125-dispcc
+
+  clocks:
+    items:
+      - description: Board XO source
+      - description: Byte clock from DSI PHY0
+      - description: Pixel clock from DSI PHY0
+      - description: Pixel clock from DSI PHY1
+      - description: Link clock from DP PHY
+      - description: VCO DIV clock from DP PHY
+      - description: AHB config clock from GCC
+
+  clock-names:
+    items:
+      - const: bi_tcxo
+      - const: dsi0_phy_pll_out_byteclk
+      - const: dsi0_phy_pll_out_dsiclk
+      - const: dsi1_phy_pll_out_dsiclk
+      - const: dp_phy_pll_link_clk
+      - const: dp_phy_pll_vco_div_clk
+      - const: cfg_ahb_clk
+
+  '#clock-cells':
+    const: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#power-domain-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,rpmcc.h>
+    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
+    clock-controller@5f00000 {
+      compatible = "qcom,sm6125-dispcc";
+      reg = <0x5f00000 0x20000>;
+      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+               <&dsi0_phy 0>,
+               <&dsi0_phy 1>,
+               <0>,
+               <&dp_phy 0>,
+               <&dp_phy 1>,
+               <&gcc GCC_DISP_AHB_CLK>;
+      clock-names = "bi_tcxo",
+                    "dsi0_phy_pll_out_byteclk",
+                    "dsi0_phy_pll_out_dsiclk",
+                    "dsi1_phy_pll_out_dsiclk",
+                    "dp_phy_pll_link_clk",
+                    "dp_phy_pll_vco_div_clk",
+                    "cfg_ahb_clk";
+      #clock-cells = <1>;
+      #power-domain-cells = <1>;
+    };
+...
diff --git a/include/dt-bindings/clock/qcom,dispcc-sm6125.h b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
new file mode 100644
index 000000000000..4ff974f4fcc3
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
+#define _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
+
+#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_INTF_CLK	5
+#define DISP_CC_MDSS_DP_AUX_CLK		6
+#define DISP_CC_MDSS_DP_AUX_CLK_SRC	7
+#define DISP_CC_MDSS_DP_CRYPTO_CLK	8
+#define DISP_CC_MDSS_DP_CRYPTO_CLK_SRC	9
+#define DISP_CC_MDSS_DP_LINK_CLK	10
+#define DISP_CC_MDSS_DP_LINK_CLK_SRC	11
+#define DISP_CC_MDSS_DP_LINK_INTF_CLK	12
+#define DISP_CC_MDSS_DP_PIXEL_CLK	13
+#define DISP_CC_MDSS_DP_PIXEL_CLK_SRC	14
+#define DISP_CC_MDSS_ESC0_CLK		15
+#define DISP_CC_MDSS_ESC0_CLK_SRC	16
+#define DISP_CC_MDSS_MDP_CLK		17
+#define DISP_CC_MDSS_MDP_CLK_SRC	18
+#define DISP_CC_MDSS_MDP_LUT_CLK	19
+#define DISP_CC_MDSS_NON_GDSC_AHB_CLK	20
+#define DISP_CC_MDSS_PCLK0_CLK		21
+#define DISP_CC_MDSS_PCLK0_CLK_SRC	22
+#define DISP_CC_MDSS_ROT_CLK		23
+#define DISP_CC_MDSS_ROT_CLK_SRC	24
+#define DISP_CC_MDSS_VSYNC_CLK		25
+#define DISP_CC_MDSS_VSYNC_CLK_SRC	26
+#define DISP_CC_XO_CLK			27
+
+/* DISP_CC GDSCR */
+#define MDSS_GDSC			0
+
+#endif
-- 
2.35.1

[PATCH v2 3/3] clk: qcom: Add display clock controller driver for SM6125

[Marijn Suijten]

From: Martin Botka <martin.botka@somainline.org>

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

Signed-off-by: Martin Botka <martin.botka@somainline.org>
---
 drivers/clk/qcom/Kconfig         |   9 +
 drivers/clk/qcom/Makefile        |   1 +
 drivers/clk/qcom/dispcc-sm6125.c | 709 +++++++++++++++++++++++++++++++
 3 files changed, 719 insertions(+)
 create mode 100644 drivers/clk/qcom/dispcc-sm6125.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 161b257da9ca..3012b8133db0 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -589,6 +589,15 @@ config SM_CAMCC_8250
 	  Support for the camera clock controller on SM8250 devices.
 	  Say Y if you want to support camera devices and camera functionality.
 
+config SM_DISPCC_6125
+	tristate "SM6125 Display Clock Controller"
+	depends on SM_GCC_6125
+	help
+	  Support for the display clock controller on Qualcomm Technologies, Inc
+	  SM6125 devices.
+	  Say Y if you want to support display devices and functionality such as
+	  splash screen
+
 config SM_DISPCC_8250
 	tristate "SM8150 and SM8250 Display Clock Controller"
 	depends on SM_GCC_8150 || SM_GCC_8250
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 3e4eb843b8d2..7d627dea665f 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o
 obj-$(CONFIG_SDX_GCC_55) += gcc-sdx55.o
 obj-$(CONFIG_SDX_GCC_65) += gcc-sdx65.o
 obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o
+obj-$(CONFIG_SM_DISPCC_6125) += dispcc-sm6125.o
 obj-$(CONFIG_SM_DISPCC_8250) += dispcc-sm8250.o
 obj-$(CONFIG_SM_GCC_6115) += gcc-sm6115.o
 obj-$(CONFIG_SM_GCC_6125) += gcc-sm6125.o
diff --git a/drivers/clk/qcom/dispcc-sm6125.c b/drivers/clk/qcom/dispcc-sm6125.c
new file mode 100644
index 000000000000..b921456a2e0d
--- /dev/null
+++ b/drivers/clk/qcom/dispcc-sm6125.c
@@ -0,0 +1,709 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021, 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 <dt-bindings/clock/qcom,dispcc-sm6125.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "common.h"
+#include "gdsc.h"
+
+enum {
+	P_BI_TCXO,
+	P_DISP_CC_PLL0_OUT_MAIN,
+	P_DP_PHY_PLL_LINK_CLK,
+	P_DP_PHY_PLL_VCO_DIV_CLK,
+	P_DSI0_PHY_PLL_OUT_BYTECLK,
+	P_DSI0_PHY_PLL_OUT_DSICLK,
+	P_DSI1_PHY_PLL_OUT_DSICLK,
+	P_GPLL0_OUT_MAIN,
+};
+
+static struct pll_vco disp_cc_pll_vco[] = {
+	{ 500000000, 1000000000, 2 },
+};
+
+static struct clk_alpha_pll disp_cc_pll0 = {
+	.offset = 0x0,
+	.vco_table = disp_cc_pll_vco,
+	.num_vco = ARRAY_SIZE(disp_cc_pll_vco),
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.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,
+		},
+	},
+};
+
+/* 768MHz configuration */
+static const struct alpha_pll_config disp_cc_pll0_config = {
+	.l = 0x28,
+	.vco_val = 0x2 << 20,
+	.vco_mask = 0x3 << 20,
+	.main_output_mask = BIT(0),
+	.config_ctl_val = 0x4001055b,
+};
+
+static const struct parent_map disp_cc_parent_map_0[] = {
+	{ P_BI_TCXO, 0 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_0[] = {
+	{ .fw_name = "bi_tcxo" },
+};
+
+static const struct parent_map disp_cc_parent_map_1[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_DP_PHY_PLL_LINK_CLK, 1 },
+	{ P_DP_PHY_PLL_VCO_DIV_CLK, 2 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_1[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "dp_phy_pll_link_clk" },
+	{ .fw_name = "dp_phy_pll_vco_div_clk" },
+};
+
+static const struct parent_map disp_cc_parent_map_2[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_DSI0_PHY_PLL_OUT_BYTECLK, 1 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_2[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "dsi0_phy_pll_out_byteclk" },
+};
+
+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 },
+};
+
+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_div_clk_src" },
+};
+
+static const struct parent_map disp_cc_parent_map_4[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_GPLL0_OUT_MAIN, 4 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_4[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },
+};
+
+static const struct parent_map disp_cc_parent_map_5[] = {
+	{ P_BI_TCXO, 0 },
+	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
+	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
+};
+
+static const struct clk_parent_data disp_cc_parent_data_5[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
+	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
+};
+
+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, 16, 0, 0),
+	F(75000000, P_GPLL0_OUT_MAIN, 8, 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_4,
+	.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_4,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
+		.ops = &clk_rcg2_shared_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = {
+	.cmd_rcgr = 0x20bc,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_2,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_byte0_clk_src",
+		.parent_data = disp_cc_parent_data_2,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_2),
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+		.ops = &clk_byte2_ops,
+	},
+};
+
+static const struct freq_tbl ftbl_disp_cc_mdss_dp_aux1_clk_src[] = {
+	F(19200000, P_BI_TCXO, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_dp_aux_clk_src = {
+	.cmd_rcgr = 0x213c,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_0,
+	.freq_tbl = ftbl_disp_cc_mdss_dp_aux1_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_dp_aux_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_dp_crypto_clk_src[] = {
+	F( 180000, P_DP_PHY_PLL_LINK_CLK,   1.5,   0,   0),
+	F( 360000, P_DP_PHY_PLL_LINK_CLK,   1.5,   0,   0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = {
+	.cmd_rcgr = 0x210c,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_1,
+	.freq_tbl = ftbl_disp_cc_mdss_dp_crypto_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_dp_crypto_clk_src",
+		.parent_data = disp_cc_parent_data_1,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
+		.flags = CLK_GET_RATE_NOCACHE,
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static const struct freq_tbl ftbl_disp_cc_mdss_dp_link_clk_src[] = {
+	F( 162000, P_DP_PHY_PLL_LINK_CLK,   1,   0,   0),
+	F( 270000, P_DP_PHY_PLL_LINK_CLK,   1,   0,   0),
+	F( 540000, P_DP_PHY_PLL_LINK_CLK,   1,   0,   0),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_dp_link_clk_src = {
+	.cmd_rcgr = 0x20f0,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_1,
+	.freq_tbl = ftbl_disp_cc_mdss_dp_link_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_dp_link_clk_src",
+		.parent_data = disp_cc_parent_data_1,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_dp_pixel_clk_src = {
+	.cmd_rcgr = 0x2124,
+	.mnd_width = 16,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_1,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_dp_pixel_clk_src",
+		.parent_data = disp_cc_parent_data_1,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+		.ops = &clk_dp_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_esc0_clk_src = {
+	.cmd_rcgr = 0x20d8,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_2,
+	.freq_tbl = ftbl_disp_cc_mdss_dp_aux1_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_esc0_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_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),
+	F(400000000, P_GPLL0_OUT_MAIN, 1.5, 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),
+		.ops = &clk_rcg2_shared_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_5,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_pclk0_clk_src",
+		.parent_data = disp_cc_parent_data_5,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_5),
+		.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(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),
+	{ }
+};
+
+static struct clk_rcg2 disp_cc_mdss_rot_clk_src = {
+	.cmd_rcgr = 0x208c,
+	.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_data = disp_cc_parent_data_3,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
+		.flags = CLK_SET_RATE_PARENT,
+		.ops = &clk_rcg2_shared_ops,
+	},
+};
+
+static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
+	.cmd_rcgr = 0x20a4,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = disp_cc_parent_map_0,
+	.freq_tbl = ftbl_disp_cc_mdss_dp_aux1_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "disp_cc_mdss_vsync_clk_src",
+		.parent_data = disp_cc_parent_data_0,
+		.num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
+		.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 = 0x2024,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2024,
+		.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 = 0x2028,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2028,
+		.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_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_dp_aux_clk = {
+	.halt_reg = 0x2040,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2040,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_dp_aux_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_dp_aux_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_dp_crypto_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_dp_crypto_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_dp_crypto_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_dp_link_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_dp_link_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_dp_link_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_dp_link_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_dp_link_intf_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_dp_link_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_GET_RATE_NOCACHE,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_dp_pixel_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_dp_pixel_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_dp_pixel_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 = 0x202c,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x202c,
+		.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 = 0x2018,
+	.halt_check = BRANCH_VOTED,
+	.clkr = {
+		.enable_reg = 0x2018,
+		.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,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_non_gdsc_ahb_clk = {
+	.halt_reg = 0x4004,
+	.halt_check = BRANCH_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_rot_clk = {
+	.halt_reg = 0x2010,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x2010,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "disp_cc_mdss_rot_clk",
+			.parent_hws = (const struct clk_hw*[]){
+				&disp_cc_mdss_rot_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch disp_cc_mdss_vsync_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_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_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",
+			.flags = CLK_IS_CRITICAL,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct gdsc mdss_gdsc = {
+	.gdscr = 0x3000,
+	.pd = {
+		.name = "mdss_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = HW_CTRL,
+};
+
+static struct clk_regmap *disp_cc_sm6125_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_INTF_CLK] = &disp_cc_mdss_byte0_intf_clk.clkr,
+	[DISP_CC_MDSS_DP_AUX_CLK] = &disp_cc_mdss_dp_aux_clk.clkr,
+	[DISP_CC_MDSS_DP_AUX_CLK_SRC] = &disp_cc_mdss_dp_aux_clk_src.clkr,
+	[DISP_CC_MDSS_DP_CRYPTO_CLK] = &disp_cc_mdss_dp_crypto_clk.clkr,
+	[DISP_CC_MDSS_DP_CRYPTO_CLK_SRC] = &disp_cc_mdss_dp_crypto_clk_src.clkr,
+	[DISP_CC_MDSS_DP_LINK_CLK] = &disp_cc_mdss_dp_link_clk.clkr,
+	[DISP_CC_MDSS_DP_LINK_CLK_SRC] = &disp_cc_mdss_dp_link_clk_src.clkr,
+	[DISP_CC_MDSS_DP_LINK_INTF_CLK] = &disp_cc_mdss_dp_link_intf_clk.clkr,
+	[DISP_CC_MDSS_DP_PIXEL_CLK] = &disp_cc_mdss_dp_pixel_clk.clkr,
+	[DISP_CC_MDSS_DP_PIXEL_CLK_SRC] = &disp_cc_mdss_dp_pixel_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_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_ROT_CLK] = &disp_cc_mdss_rot_clk.clkr,
+	[DISP_CC_MDSS_ROT_CLK_SRC] = &disp_cc_mdss_rot_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_XO_CLK] = &disp_cc_xo_clk.clkr,
+};
+
+static struct gdsc *disp_cc_sm6125_gdscs[] = {
+	[MDSS_GDSC] = &mdss_gdsc,
+};
+
+static const struct regmap_config disp_cc_sm6125_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_sm6125_desc = {
+	.config = &disp_cc_sm6125_regmap_config,
+	.clks = disp_cc_sm6125_clocks,
+	.num_clks = ARRAY_SIZE(disp_cc_sm6125_clocks),
+	.gdscs = disp_cc_sm6125_gdscs,
+	.num_gdscs = ARRAY_SIZE(disp_cc_sm6125_gdscs),
+};
+
+static const struct of_device_id disp_cc_sm6125_match_table[] = {
+	{ .compatible = "qcom,dispcc-sm6125" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, disp_cc_sm6125_match_table);
+
+static int disp_cc_sm6125_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+
+	regmap = qcom_cc_map(pdev, &disp_cc_sm6125_desc);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	clk_alpha_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
+
+	return qcom_cc_really_probe(pdev, &disp_cc_sm6125_desc, regmap);
+}
+
+static struct platform_driver disp_cc_sm6125_driver = {
+	.probe = disp_cc_sm6125_probe,
+	.driver = {
+		.name = "disp_cc-sm6125",
+		.of_match_table = disp_cc_sm6125_match_table,
+	},
+};
+
+static int __init disp_cc_sm6125_init(void)
+{
+	return platform_driver_register(&disp_cc_sm6125_driver);
+}
+subsys_initcall(disp_cc_sm6125_init);
+
+static void __exit disp_cc_sm6125_exit(void)
+{
+	platform_driver_unregister(&disp_cc_sm6125_driver);
+}
+module_exit(disp_cc_sm6125_exit);
+
+MODULE_DESCRIPTION("QTI DISPCC SM6125 Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.35.1

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Marijn Suijten]

On 2022-02-26 21:09:10, Marijn Suijten wrote:
> From: Martin Botka <martin.botka@somainline.org>
> 
> Add device tree bindings for display clock controller for
> Qualcomm Technology Inc's SM6125 SoC.
> 
> Signed-off-by: Martin Botka <martin.botka@somainline.org>

This of course lacks the mandatory sign-off after getting permission to
apply my own review and mailing-list review, and resending the patch:

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

> ---
>  .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
>  .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
>  2 files changed, 128 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>  create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> new file mode 100644
> index 000000000000..3465042d0d9f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Display Clock Controller Binding for SM6125
> +
> +maintainers:
> +  - Martin Botka <martin.botka@somainline.org>
> +
> +description: |
> +  Qualcomm display clock control module which supports the clocks and
> +  power domains on SM6125.
> +
> +  See also:
> +    dt-bindings/clock/qcom,dispcc-sm6125.h
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sm6125-dispcc
> +
> +  clocks:
> +    items:
> +      - description: Board XO source
> +      - description: Byte clock from DSI PHY0
> +      - description: Pixel clock from DSI PHY0
> +      - description: Pixel clock from DSI PHY1
> +      - description: Link clock from DP PHY
> +      - description: VCO DIV clock from DP PHY
> +      - description: AHB config clock from GCC
> +
> +  clock-names:
> +    items:
> +      - const: bi_tcxo
> +      - const: dsi0_phy_pll_out_byteclk
> +      - const: dsi0_phy_pll_out_dsiclk
> +      - const: dsi1_phy_pll_out_dsiclk
> +      - const: dp_phy_pll_link_clk
> +      - const: dp_phy_pll_vco_div_clk
> +      - const: cfg_ahb_clk
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#power-domain-cells':
> +    const: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#power-domain-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,rpmcc.h>
> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> +    clock-controller@5f00000 {
> +      compatible = "qcom,sm6125-dispcc";
> +      reg = <0x5f00000 0x20000>;
> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +               <&dsi0_phy 0>,
> +               <&dsi0_phy 1>,
> +               <0>,
> +               <&dp_phy 0>,
> +               <&dp_phy 1>,
> +               <&gcc GCC_DISP_AHB_CLK>;
> +      clock-names = "bi_tcxo",
> +                    "dsi0_phy_pll_out_byteclk",
> +                    "dsi0_phy_pll_out_dsiclk",
> +                    "dsi1_phy_pll_out_dsiclk",
> +                    "dp_phy_pll_link_clk",
> +                    "dp_phy_pll_vco_div_clk",
> +                    "cfg_ahb_clk";
> +      #clock-cells = <1>;
> +      #power-domain-cells = <1>;
> +    };
> +...
> diff --git a/include/dt-bindings/clock/qcom,dispcc-sm6125.h b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
> new file mode 100644
> index 000000000000..4ff974f4fcc3
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
> +#define _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
> +
> +#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_INTF_CLK	5
> +#define DISP_CC_MDSS_DP_AUX_CLK		6
> +#define DISP_CC_MDSS_DP_AUX_CLK_SRC	7
> +#define DISP_CC_MDSS_DP_CRYPTO_CLK	8
> +#define DISP_CC_MDSS_DP_CRYPTO_CLK_SRC	9
> +#define DISP_CC_MDSS_DP_LINK_CLK	10
> +#define DISP_CC_MDSS_DP_LINK_CLK_SRC	11
> +#define DISP_CC_MDSS_DP_LINK_INTF_CLK	12
> +#define DISP_CC_MDSS_DP_PIXEL_CLK	13
> +#define DISP_CC_MDSS_DP_PIXEL_CLK_SRC	14
> +#define DISP_CC_MDSS_ESC0_CLK		15
> +#define DISP_CC_MDSS_ESC0_CLK_SRC	16
> +#define DISP_CC_MDSS_MDP_CLK		17
> +#define DISP_CC_MDSS_MDP_CLK_SRC	18
> +#define DISP_CC_MDSS_MDP_LUT_CLK	19
> +#define DISP_CC_MDSS_NON_GDSC_AHB_CLK	20
> +#define DISP_CC_MDSS_PCLK0_CLK		21
> +#define DISP_CC_MDSS_PCLK0_CLK_SRC	22
> +#define DISP_CC_MDSS_ROT_CLK		23
> +#define DISP_CC_MDSS_ROT_CLK_SRC	24
> +#define DISP_CC_MDSS_VSYNC_CLK		25
> +#define DISP_CC_MDSS_VSYNC_CLK_SRC	26
> +#define DISP_CC_XO_CLK			27
> +
> +/* DISP_CC GDSCR */
> +#define MDSS_GDSC			0
> +
> +#endif
> -- 
> 2.35.1
> 

Re: [PATCH v2 3/3] clk: qcom: Add display clock controller driver for SM6125

[Marijn Suijten]

On 2022-02-26 21:09:11, Marijn Suijten wrote:
> From: Martin Botka <martin.botka@somainline.org>
> 
> Add support for the display clock controller found on SM6125
> based devices. This allows display drivers to probe and
> control their clocks.
> 
> Signed-off-by: Martin Botka <martin.botka@somainline.org>

This of course lacks the mandatory sign-off after getting permission to
apply my own review and mailing-list review, and resending the patch:

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

> ---
>  drivers/clk/qcom/Kconfig         |   9 +
>  drivers/clk/qcom/Makefile        |   1 +
>  drivers/clk/qcom/dispcc-sm6125.c | 709 +++++++++++++++++++++++++++++++
>  3 files changed, 719 insertions(+)
>  create mode 100644 drivers/clk/qcom/dispcc-sm6125.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 161b257da9ca..3012b8133db0 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -589,6 +589,15 @@ config SM_CAMCC_8250
>  	  Support for the camera clock controller on SM8250 devices.
>  	  Say Y if you want to support camera devices and camera functionality.
>  
> +config SM_DISPCC_6125
> +	tristate "SM6125 Display Clock Controller"
> +	depends on SM_GCC_6125
> +	help
> +	  Support for the display clock controller on Qualcomm Technologies, Inc
> +	  SM6125 devices.
> +	  Say Y if you want to support display devices and functionality such as
> +	  splash screen
> +
>  config SM_DISPCC_8250
>  	tristate "SM8150 and SM8250 Display Clock Controller"
>  	depends on SM_GCC_8150 || SM_GCC_8250
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 3e4eb843b8d2..7d627dea665f 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -86,6 +86,7 @@ obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o
>  obj-$(CONFIG_SDX_GCC_55) += gcc-sdx55.o
>  obj-$(CONFIG_SDX_GCC_65) += gcc-sdx65.o
>  obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o
> +obj-$(CONFIG_SM_DISPCC_6125) += dispcc-sm6125.o
>  obj-$(CONFIG_SM_DISPCC_8250) += dispcc-sm8250.o
>  obj-$(CONFIG_SM_GCC_6115) += gcc-sm6115.o
>  obj-$(CONFIG_SM_GCC_6125) += gcc-sm6125.o
> diff --git a/drivers/clk/qcom/dispcc-sm6125.c b/drivers/clk/qcom/dispcc-sm6125.c
> new file mode 100644
> index 000000000000..b921456a2e0d
> --- /dev/null
> +++ b/drivers/clk/qcom/dispcc-sm6125.c
> @@ -0,0 +1,709 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2021, 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 <dt-bindings/clock/qcom,dispcc-sm6125.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "common.h"
> +#include "gdsc.h"
> +
> +enum {
> +	P_BI_TCXO,
> +	P_DISP_CC_PLL0_OUT_MAIN,
> +	P_DP_PHY_PLL_LINK_CLK,
> +	P_DP_PHY_PLL_VCO_DIV_CLK,
> +	P_DSI0_PHY_PLL_OUT_BYTECLK,
> +	P_DSI0_PHY_PLL_OUT_DSICLK,
> +	P_DSI1_PHY_PLL_OUT_DSICLK,
> +	P_GPLL0_OUT_MAIN,
> +};
> +
> +static struct pll_vco disp_cc_pll_vco[] = {
> +	{ 500000000, 1000000000, 2 },
> +};
> +
> +static struct clk_alpha_pll disp_cc_pll0 = {
> +	.offset = 0x0,
> +	.vco_table = disp_cc_pll_vco,
> +	.num_vco = ARRAY_SIZE(disp_cc_pll_vco),
> +	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> +	.flags = SUPPORTS_DYNAMIC_UPDATE,
> +	.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,
> +		},
> +	},
> +};
> +
> +/* 768MHz configuration */
> +static const struct alpha_pll_config disp_cc_pll0_config = {
> +	.l = 0x28,
> +	.vco_val = 0x2 << 20,
> +	.vco_mask = 0x3 << 20,
> +	.main_output_mask = BIT(0),
> +	.config_ctl_val = 0x4001055b,
> +};
> +
> +static const struct parent_map disp_cc_parent_map_0[] = {
> +	{ P_BI_TCXO, 0 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_0[] = {
> +	{ .fw_name = "bi_tcxo" },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_1[] = {
> +	{ P_BI_TCXO, 0 },
> +	{ P_DP_PHY_PLL_LINK_CLK, 1 },
> +	{ P_DP_PHY_PLL_VCO_DIV_CLK, 2 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_1[] = {
> +	{ .fw_name = "bi_tcxo" },
> +	{ .fw_name = "dp_phy_pll_link_clk" },
> +	{ .fw_name = "dp_phy_pll_vco_div_clk" },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_2[] = {
> +	{ P_BI_TCXO, 0 },
> +	{ P_DSI0_PHY_PLL_OUT_BYTECLK, 1 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_2[] = {
> +	{ .fw_name = "bi_tcxo" },
> +	{ .fw_name = "dsi0_phy_pll_out_byteclk" },
> +};
> +
> +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 },
> +};
> +
> +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_div_clk_src" },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_4[] = {
> +	{ P_BI_TCXO, 0 },
> +	{ P_GPLL0_OUT_MAIN, 4 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_4[] = {
> +	{ .fw_name = "bi_tcxo" },
> +	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },
> +};
> +
> +static const struct parent_map disp_cc_parent_map_5[] = {
> +	{ P_BI_TCXO, 0 },
> +	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
> +	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
> +};
> +
> +static const struct clk_parent_data disp_cc_parent_data_5[] = {
> +	{ .fw_name = "bi_tcxo" },
> +	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
> +	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
> +};
> +
> +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, 16, 0, 0),
> +	F(75000000, P_GPLL0_OUT_MAIN, 8, 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_4,
> +	.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_4,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
> +		.ops = &clk_rcg2_shared_ops,
> +	},
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = {
> +	.cmd_rcgr = 0x20bc,
> +	.mnd_width = 0,
> +	.hid_width = 5,
> +	.parent_map = disp_cc_parent_map_2,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_byte0_clk_src",
> +		.parent_data = disp_cc_parent_data_2,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_2),
> +		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +		.ops = &clk_byte2_ops,
> +	},
> +};
> +
> +static const struct freq_tbl ftbl_disp_cc_mdss_dp_aux1_clk_src[] = {
> +	F(19200000, P_BI_TCXO, 1, 0, 0),
> +	{ }
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_dp_aux_clk_src = {
> +	.cmd_rcgr = 0x213c,
> +	.mnd_width = 0,
> +	.hid_width = 5,
> +	.parent_map = disp_cc_parent_map_0,
> +	.freq_tbl = ftbl_disp_cc_mdss_dp_aux1_clk_src,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_dp_aux_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_dp_crypto_clk_src[] = {
> +	F( 180000, P_DP_PHY_PLL_LINK_CLK,   1.5,   0,   0),
> +	F( 360000, P_DP_PHY_PLL_LINK_CLK,   1.5,   0,   0),
> +	{ }
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = {
> +	.cmd_rcgr = 0x210c,
> +	.mnd_width = 0,
> +	.hid_width = 5,
> +	.parent_map = disp_cc_parent_map_1,
> +	.freq_tbl = ftbl_disp_cc_mdss_dp_crypto_clk_src,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_dp_crypto_clk_src",
> +		.parent_data = disp_cc_parent_data_1,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
> +		.flags = CLK_GET_RATE_NOCACHE,
> +		.ops = &clk_rcg2_ops,
> +	},
> +};
> +
> +static const struct freq_tbl ftbl_disp_cc_mdss_dp_link_clk_src[] = {
> +	F( 162000, P_DP_PHY_PLL_LINK_CLK,   1,   0,   0),
> +	F( 270000, P_DP_PHY_PLL_LINK_CLK,   1,   0,   0),
> +	F( 540000, P_DP_PHY_PLL_LINK_CLK,   1,   0,   0),
> +	{ }
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_dp_link_clk_src = {
> +	.cmd_rcgr = 0x20f0,
> +	.mnd_width = 0,
> +	.hid_width = 5,
> +	.parent_map = disp_cc_parent_map_1,
> +	.freq_tbl = ftbl_disp_cc_mdss_dp_link_clk_src,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_dp_link_clk_src",
> +		.parent_data = disp_cc_parent_data_1,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
> +		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +		.ops = &clk_rcg2_ops,
> +	},
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_dp_pixel_clk_src = {
> +	.cmd_rcgr = 0x2124,
> +	.mnd_width = 16,
> +	.hid_width = 5,
> +	.parent_map = disp_cc_parent_map_1,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_dp_pixel_clk_src",
> +		.parent_data = disp_cc_parent_data_1,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
> +		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +		.ops = &clk_dp_ops,
> +	},
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_esc0_clk_src = {
> +	.cmd_rcgr = 0x20d8,
> +	.mnd_width = 0,
> +	.hid_width = 5,
> +	.parent_map = disp_cc_parent_map_2,
> +	.freq_tbl = ftbl_disp_cc_mdss_dp_aux1_clk_src,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_esc0_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_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),
> +	F(400000000, P_GPLL0_OUT_MAIN, 1.5, 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),
> +		.ops = &clk_rcg2_shared_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_5,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_pclk0_clk_src",
> +		.parent_data = disp_cc_parent_data_5,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_5),
> +		.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(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),
> +	{ }
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_rot_clk_src = {
> +	.cmd_rcgr = 0x208c,
> +	.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_data = disp_cc_parent_data_3,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> +		.flags = CLK_SET_RATE_PARENT,
> +		.ops = &clk_rcg2_shared_ops,
> +	},
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
> +	.cmd_rcgr = 0x20a4,
> +	.mnd_width = 0,
> +	.hid_width = 5,
> +	.parent_map = disp_cc_parent_map_0,
> +	.freq_tbl = ftbl_disp_cc_mdss_dp_aux1_clk_src,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "disp_cc_mdss_vsync_clk_src",
> +		.parent_data = disp_cc_parent_data_0,
> +		.num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> +		.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 = 0x2024,
> +	.halt_check = BRANCH_HALT,
> +	.clkr = {
> +		.enable_reg = 0x2024,
> +		.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 = 0x2028,
> +	.halt_check = BRANCH_HALT,
> +	.clkr = {
> +		.enable_reg = 0x2028,
> +		.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_clk_src.clkr.hw,
> +			},
> +			.num_parents = 1,
> +			.flags = CLK_GET_RATE_NOCACHE,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static struct clk_branch disp_cc_mdss_dp_aux_clk = {
> +	.halt_reg = 0x2040,
> +	.halt_check = BRANCH_HALT,
> +	.clkr = {
> +		.enable_reg = 0x2040,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "disp_cc_mdss_dp_aux_clk",
> +			.parent_hws = (const struct clk_hw*[]){
> +				&disp_cc_mdss_dp_aux_clk_src.clkr.hw,
> +			},
> +			.num_parents = 1,
> +			.flags = CLK_SET_RATE_PARENT,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static struct clk_branch disp_cc_mdss_dp_crypto_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_dp_crypto_clk",
> +			.parent_hws = (const struct clk_hw*[]){
> +				&disp_cc_mdss_dp_crypto_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_dp_link_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_dp_link_clk",
> +			.parent_hws = (const struct clk_hw*[]){
> +				&disp_cc_mdss_dp_link_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_dp_link_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_dp_link_intf_clk",
> +			.parent_hws = (const struct clk_hw*[]){
> +				&disp_cc_mdss_dp_link_clk_src.clkr.hw,
> +			},
> +			.num_parents = 1,
> +			.flags = CLK_GET_RATE_NOCACHE,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static struct clk_branch disp_cc_mdss_dp_pixel_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_dp_pixel_clk",
> +			.parent_hws = (const struct clk_hw*[]){
> +				&disp_cc_mdss_dp_pixel_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 = 0x202c,
> +	.halt_check = BRANCH_HALT,
> +	.clkr = {
> +		.enable_reg = 0x202c,
> +		.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 = 0x2018,
> +	.halt_check = BRANCH_VOTED,
> +	.clkr = {
> +		.enable_reg = 0x2018,
> +		.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,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static struct clk_branch disp_cc_mdss_non_gdsc_ahb_clk = {
> +	.halt_reg = 0x4004,
> +	.halt_check = BRANCH_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_rot_clk = {
> +	.halt_reg = 0x2010,
> +	.halt_check = BRANCH_HALT,
> +	.clkr = {
> +		.enable_reg = 0x2010,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "disp_cc_mdss_rot_clk",
> +			.parent_hws = (const struct clk_hw*[]){
> +				&disp_cc_mdss_rot_clk_src.clkr.hw,
> +			},
> +			.num_parents = 1,
> +			.flags = CLK_SET_RATE_PARENT,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static struct clk_branch disp_cc_mdss_vsync_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_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_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",
> +			.flags = CLK_IS_CRITICAL,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static struct gdsc mdss_gdsc = {
> +	.gdscr = 0x3000,
> +	.pd = {
> +		.name = "mdss_gdsc",
> +	},
> +	.pwrsts = PWRSTS_OFF_ON,
> +	.flags = HW_CTRL,
> +};
> +
> +static struct clk_regmap *disp_cc_sm6125_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_INTF_CLK] = &disp_cc_mdss_byte0_intf_clk.clkr,
> +	[DISP_CC_MDSS_DP_AUX_CLK] = &disp_cc_mdss_dp_aux_clk.clkr,
> +	[DISP_CC_MDSS_DP_AUX_CLK_SRC] = &disp_cc_mdss_dp_aux_clk_src.clkr,
> +	[DISP_CC_MDSS_DP_CRYPTO_CLK] = &disp_cc_mdss_dp_crypto_clk.clkr,
> +	[DISP_CC_MDSS_DP_CRYPTO_CLK_SRC] = &disp_cc_mdss_dp_crypto_clk_src.clkr,
> +	[DISP_CC_MDSS_DP_LINK_CLK] = &disp_cc_mdss_dp_link_clk.clkr,
> +	[DISP_CC_MDSS_DP_LINK_CLK_SRC] = &disp_cc_mdss_dp_link_clk_src.clkr,
> +	[DISP_CC_MDSS_DP_LINK_INTF_CLK] = &disp_cc_mdss_dp_link_intf_clk.clkr,
> +	[DISP_CC_MDSS_DP_PIXEL_CLK] = &disp_cc_mdss_dp_pixel_clk.clkr,
> +	[DISP_CC_MDSS_DP_PIXEL_CLK_SRC] = &disp_cc_mdss_dp_pixel_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_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_ROT_CLK] = &disp_cc_mdss_rot_clk.clkr,
> +	[DISP_CC_MDSS_ROT_CLK_SRC] = &disp_cc_mdss_rot_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_XO_CLK] = &disp_cc_xo_clk.clkr,
> +};
> +
> +static struct gdsc *disp_cc_sm6125_gdscs[] = {
> +	[MDSS_GDSC] = &mdss_gdsc,
> +};
> +
> +static const struct regmap_config disp_cc_sm6125_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_sm6125_desc = {
> +	.config = &disp_cc_sm6125_regmap_config,
> +	.clks = disp_cc_sm6125_clocks,
> +	.num_clks = ARRAY_SIZE(disp_cc_sm6125_clocks),
> +	.gdscs = disp_cc_sm6125_gdscs,
> +	.num_gdscs = ARRAY_SIZE(disp_cc_sm6125_gdscs),
> +};
> +
> +static const struct of_device_id disp_cc_sm6125_match_table[] = {
> +	{ .compatible = "qcom,dispcc-sm6125" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, disp_cc_sm6125_match_table);
> +
> +static int disp_cc_sm6125_probe(struct platform_device *pdev)
> +{
> +	struct regmap *regmap;
> +
> +	regmap = qcom_cc_map(pdev, &disp_cc_sm6125_desc);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	clk_alpha_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
> +
> +	return qcom_cc_really_probe(pdev, &disp_cc_sm6125_desc, regmap);
> +}
> +
> +static struct platform_driver disp_cc_sm6125_driver = {
> +	.probe = disp_cc_sm6125_probe,
> +	.driver = {
> +		.name = "disp_cc-sm6125",
> +		.of_match_table = disp_cc_sm6125_match_table,
> +	},
> +};
> +
> +static int __init disp_cc_sm6125_init(void)
> +{
> +	return platform_driver_register(&disp_cc_sm6125_driver);
> +}
> +subsys_initcall(disp_cc_sm6125_init);
> +
> +static void __exit disp_cc_sm6125_exit(void)
> +{
> +	platform_driver_unregister(&disp_cc_sm6125_driver);
> +}
> +module_exit(disp_cc_sm6125_exit);
> +
> +MODULE_DESCRIPTION("QTI DISPCC SM6125 Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.35.1
> 

Re: [PATCH v2 1/3] clk: qcom: Fix sorting of SDX_GCC_65 in Makefile and Kconfig

[Dmitry Baryshkov]

On Sun, 27 Feb 2022 at 02:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> In order to keep at least the list of `CONFIG_SM_` drivers sorted
> alphabetically, SDX_GCC_65 should have been moved one line up.  This in
> turn makes it easier and cleaner to add the followup SM_DISPCC_6125
> driver in the right place, right before SM_DISPCC_8250.
>
> Fixes: d79afa201328 ("clk: qcom: Add SDX65 GCC support")
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/clk/qcom/Kconfig  | 14 +++++++-------
>  drivers/clk/qcom/Makefile |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index f5b54bfc992f..161b257da9ca 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -574,13 +574,6 @@ config SDX_GCC_55
>           Say Y if you want to use peripheral devices such as UART,
>           SPI, I2C, USB, SD/UFS, PCIe etc.
>
> -config SM_CAMCC_8250
> -       tristate "SM8250 Camera Clock Controller"
> -       select SM_GCC_8250
> -       help
> -         Support for the camera clock controller on SM8250 devices.
> -         Say Y if you want to support camera devices and camera functionality.
> -
>  config SDX_GCC_65
>         tristate "SDX65 Global Clock Controller"
>         select QCOM_GDSC
> @@ -589,6 +582,13 @@ config SDX_GCC_65
>           Say Y if you want to use peripheral devices such as UART,
>           SPI, I2C, USB, SD/UFS, PCIe etc.
>
> +config SM_CAMCC_8250
> +       tristate "SM8250 Camera Clock Controller"
> +       select SM_GCC_8250
> +       help
> +         Support for the camera clock controller on SM8250 devices.
> +         Say Y if you want to support camera devices and camera functionality.
> +
>  config SM_DISPCC_8250
>         tristate "SM8150 and SM8250 Display Clock Controller"
>         depends on SM_GCC_8150 || SM_GCC_8250
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index d96d6793fc7d..3e4eb843b8d2 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -84,8 +84,8 @@ obj-$(CONFIG_SDM_GPUCC_845) += gpucc-sdm845.o
>  obj-$(CONFIG_SDM_LPASSCC_845) += lpasscc-sdm845.o
>  obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o
>  obj-$(CONFIG_SDX_GCC_55) += gcc-sdx55.o
> -obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o
>  obj-$(CONFIG_SDX_GCC_65) += gcc-sdx65.o
> +obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o
>  obj-$(CONFIG_SM_DISPCC_8250) += dispcc-sm8250.o
>  obj-$(CONFIG_SM_GCC_6115) += gcc-sm6115.o
>  obj-$(CONFIG_SM_GCC_6125) += gcc-sm6125.o
> --
> 2.35.1
>


-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Krzysztof Kozlowski]

On 26/02/2022 21:09, Marijn Suijten wrote:
> From: Martin Botka <martin.botka@somainline.org>
> 
> Add device tree bindings for display clock controller for
> Qualcomm Technology Inc's SM6125 SoC.
> 
> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> ---
>  .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
>  .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
>  2 files changed, 128 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>  create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> new file mode 100644
> index 000000000000..3465042d0d9f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Display Clock Controller Binding for SM6125
> +
> +maintainers:
> +  - Martin Botka <martin.botka@somainline.org>
> +
> +description: |
> +  Qualcomm display clock control module which supports the clocks and
> +  power domains on SM6125.
> +
> +  See also:
> +    dt-bindings/clock/qcom,dispcc-sm6125.h
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sm6125-dispcc
> +
> +  clocks:
> +    items:
> +      - description: Board XO source
> +      - description: Byte clock from DSI PHY0
> +      - description: Pixel clock from DSI PHY0
> +      - description: Pixel clock from DSI PHY1
> +      - description: Link clock from DP PHY
> +      - description: VCO DIV clock from DP PHY
> +      - description: AHB config clock from GCC
> +
> +  clock-names:
> +    items:
> +      - const: bi_tcxo
> +      - const: dsi0_phy_pll_out_byteclk
> +      - const: dsi0_phy_pll_out_dsiclk
> +      - const: dsi1_phy_pll_out_dsiclk
> +      - const: dp_phy_pll_link_clk
> +      - const: dp_phy_pll_vco_div_clk
> +      - const: cfg_ahb_clk
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#power-domain-cells':
> +    const: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#power-domain-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,rpmcc.h>
> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> +    clock-controller@5f00000 {
> +      compatible = "qcom,sm6125-dispcc";
> +      reg = <0x5f00000 0x20000>;
> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +               <&dsi0_phy 0>,
> +               <&dsi0_phy 1>,
> +               <0>,

This does not look like a valid phandle. This clock is required, isn't it?


Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Dmitry Baryshkov]

On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
> On 26/02/2022 21:09, Marijn Suijten wrote:
>> From: Martin Botka <martin.botka@somainline.org>
>>
>> Add device tree bindings for display clock controller for
>> Qualcomm Technology Inc's SM6125 SoC.
>>
>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>> ---
>>   .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
>>   .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
>>   2 files changed, 128 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>   create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>> new file mode 100644
>> index 000000000000..3465042d0d9f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>> @@ -0,0 +1,87 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Display Clock Controller Binding for SM6125
>> +
>> +maintainers:
>> +  - Martin Botka <martin.botka@somainline.org>
>> +
>> +description: |
>> +  Qualcomm display clock control module which supports the clocks and
>> +  power domains on SM6125.
>> +
>> +  See also:
>> +    dt-bindings/clock/qcom,dispcc-sm6125.h
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,sm6125-dispcc
>> +
>> +  clocks:
>> +    items:
>> +      - description: Board XO source
>> +      - description: Byte clock from DSI PHY0
>> +      - description: Pixel clock from DSI PHY0
>> +      - description: Pixel clock from DSI PHY1
>> +      - description: Link clock from DP PHY
>> +      - description: VCO DIV clock from DP PHY
>> +      - description: AHB config clock from GCC
>> +
>> +  clock-names:
>> +    items:
>> +      - const: bi_tcxo
>> +      - const: dsi0_phy_pll_out_byteclk
>> +      - const: dsi0_phy_pll_out_dsiclk
>> +      - const: dsi1_phy_pll_out_dsiclk
>> +      - const: dp_phy_pll_link_clk
>> +      - const: dp_phy_pll_vco_div_clk
>> +      - const: cfg_ahb_clk
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +  '#power-domain-cells':
>> +    const: 1
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - '#clock-cells'
>> +  - '#power-domain-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/qcom,rpmcc.h>
>> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>> +    clock-controller@5f00000 {
>> +      compatible = "qcom,sm6125-dispcc";
>> +      reg = <0x5f00000 0x20000>;
>> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>> +               <&dsi0_phy 0>,
>> +               <&dsi0_phy 1>,
>> +               <0>,
> 
> This does not look like a valid phandle. This clock is required, isn't it?

Not, it's not required for general dispcc support.
dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc 
clocks. However if support for DP is not enabled, the dispcc can work 
w/o DP phy clock. Thus we typically add 0 phandles as placeholders for 
DSI/DP clock sources and populate them as support for respective 
interfaces gets implemented.

> 
> 
> Best regards,
> Krzysztof


-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Krzysztof Kozlowski]

On 27/02/2022 22:43, Dmitry Baryshkov wrote:
> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
>> On 26/02/2022 21:09, Marijn Suijten wrote:
>>> From: Martin Botka <martin.botka@somainline.org>
>>>
>>> Add device tree bindings for display clock controller for
>>> Qualcomm Technology Inc's SM6125 SoC.
>>>
>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>> ---
>>>   .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
>>>   .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
>>>   2 files changed, 128 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>   create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> new file mode 100644
>>> index 000000000000..3465042d0d9f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> @@ -0,0 +1,87 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm Display Clock Controller Binding for SM6125
>>> +
>>> +maintainers:
>>> +  - Martin Botka <martin.botka@somainline.org>
>>> +
>>> +description: |
>>> +  Qualcomm display clock control module which supports the clocks and
>>> +  power domains on SM6125.
>>> +
>>> +  See also:
>>> +    dt-bindings/clock/qcom,dispcc-sm6125.h
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - qcom,sm6125-dispcc
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: Board XO source
>>> +      - description: Byte clock from DSI PHY0
>>> +      - description: Pixel clock from DSI PHY0
>>> +      - description: Pixel clock from DSI PHY1
>>> +      - description: Link clock from DP PHY
>>> +      - description: VCO DIV clock from DP PHY
>>> +      - description: AHB config clock from GCC
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: bi_tcxo
>>> +      - const: dsi0_phy_pll_out_byteclk
>>> +      - const: dsi0_phy_pll_out_dsiclk
>>> +      - const: dsi1_phy_pll_out_dsiclk
>>> +      - const: dp_phy_pll_link_clk
>>> +      - const: dp_phy_pll_vco_div_clk
>>> +      - const: cfg_ahb_clk
>>> +
>>> +  '#clock-cells':
>>> +    const: 1
>>> +
>>> +  '#power-domain-cells':
>>> +    const: 1
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - clock-names
>>> +  - '#clock-cells'
>>> +  - '#power-domain-cells'
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/qcom,rpmcc.h>
>>> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>>> +    clock-controller@5f00000 {
>>> +      compatible = "qcom,sm6125-dispcc";
>>> +      reg = <0x5f00000 0x20000>;
>>> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>>> +               <&dsi0_phy 0>,
>>> +               <&dsi0_phy 1>,
>>> +               <0>,
>>
>> This does not look like a valid phandle. This clock is required, isn't it?
> 
> Not, it's not required for general dispcc support.
> dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc 
> clocks. However if support for DP is not enabled, the dispcc can work 
> w/o DP phy clock. Thus we typically add 0 phandles as placeholders for 
> DSI/DP clock sources and populate them as support for respective 
> interfaces gets implemented.
> 

Then the clock is optional, isn't it? While not modeling it as optional?


Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Marijn Suijten]

On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
> > On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
> >> On 26/02/2022 21:09, Marijn Suijten wrote:
> >>> From: Martin Botka <martin.botka@somainline.org>
> >>>
> >>> Add device tree bindings for display clock controller for
> >>> Qualcomm Technology Inc's SM6125 SoC.
> >>>
> >>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> >>> ---
> >>>   .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
> >>>   .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
> >>>   2 files changed, 128 insertions(+)
> >>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>>   create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> new file mode 100644
> >>> index 000000000000..3465042d0d9f
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> @@ -0,0 +1,87 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Qualcomm Display Clock Controller Binding for SM6125
> >>> +
> >>> +maintainers:
> >>> +  - Martin Botka <martin.botka@somainline.org>
> >>> +
> >>> +description: |
> >>> +  Qualcomm display clock control module which supports the clocks and
> >>> +  power domains on SM6125.
> >>> +
> >>> +  See also:
> >>> +    dt-bindings/clock/qcom,dispcc-sm6125.h
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - qcom,sm6125-dispcc
> >>> +
> >>> +  clocks:
> >>> +    items:
> >>> +      - description: Board XO source
> >>> +      - description: Byte clock from DSI PHY0
> >>> +      - description: Pixel clock from DSI PHY0
> >>> +      - description: Pixel clock from DSI PHY1
> >>> +      - description: Link clock from DP PHY
> >>> +      - description: VCO DIV clock from DP PHY
> >>> +      - description: AHB config clock from GCC
> >>> +
> >>> +  clock-names:
> >>> +    items:
> >>> +      - const: bi_tcxo
> >>> +      - const: dsi0_phy_pll_out_byteclk
> >>> +      - const: dsi0_phy_pll_out_dsiclk
> >>> +      - const: dsi1_phy_pll_out_dsiclk
> >>> +      - const: dp_phy_pll_link_clk
> >>> +      - const: dp_phy_pll_vco_div_clk
> >>> +      - const: cfg_ahb_clk
> >>> +
> >>> +  '#clock-cells':
> >>> +    const: 1
> >>> +
> >>> +  '#power-domain-cells':
> >>> +    const: 1
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - clocks
> >>> +  - clock-names
> >>> +  - '#clock-cells'
> >>> +  - '#power-domain-cells'
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    #include <dt-bindings/clock/qcom,rpmcc.h>
> >>> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> >>> +    clock-controller@5f00000 {
> >>> +      compatible = "qcom,sm6125-dispcc";
> >>> +      reg = <0x5f00000 0x20000>;
> >>> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> >>> +               <&dsi0_phy 0>,
> >>> +               <&dsi0_phy 1>,
> >>> +               <0>,
> >>
> >> This does not look like a valid phandle. This clock is required, isn't it?

I remember it being used like this before, though upon closer inspection
only qcom,gcc-msm8998.yaml uses it as example.

The clock should be optional, in that case it is perhaps desired to omit
it from clock-names instead, or pretend there's a `dsi1_phy 1`?

> > 
> > Not, it's not required for general dispcc support.
> > dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc 
> > clocks. However if support for DP is not enabled, the dispcc can work 
> > w/o DP phy clock. Thus we typically add 0 phandles as placeholders for 

Is there any semantic difference between omitting the clock from DT (in
clocks= /and/ clock-names=) or setting it to a 0 phandle?

> > DSI/DP clock sources and populate them as support for respective 
> > interfaces gets implemented.
> > 
> 
> Then the clock is optional, isn't it? While not modeling it as optional?

It looks like this should be modelled using minItems: then, and
"optional" text/comment? Other clocks are optional as well, we don't
have DSI 1 in downstream SM6125 DT sources and haven't added the DP PLL
in our to-be-upstreamed mainline tree yet.

- Marijn

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Krzysztof Kozlowski]

On 02/03/2022 13:54, Marijn Suijten wrote:
> On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
>> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
>>> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
>>>> On 26/02/2022 21:09, Marijn Suijten wrote:
>>>>> From: Martin Botka <martin.botka@somainline.org>
>>>>>
>>>>> Add device tree bindings for display clock controller for
>>>>> Qualcomm Technology Inc's SM6125 SoC.
>>>>>
>>>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>>>> ---
>>>>>   .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
>>>>>   .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
>>>>>   2 files changed, 128 insertions(+)
>>>>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>>   create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..3465042d0d9f
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>> @@ -0,0 +1,87 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Qualcomm Display Clock Controller Binding for SM6125
>>>>> +
>>>>> +maintainers:
>>>>> +  - Martin Botka <martin.botka@somainline.org>
>>>>> +
>>>>> +description: |
>>>>> +  Qualcomm display clock control module which supports the clocks and
>>>>> +  power domains on SM6125.
>>>>> +
>>>>> +  See also:
>>>>> +    dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - qcom,sm6125-dispcc
>>>>> +
>>>>> +  clocks:
>>>>> +    items:
>>>>> +      - description: Board XO source
>>>>> +      - description: Byte clock from DSI PHY0
>>>>> +      - description: Pixel clock from DSI PHY0
>>>>> +      - description: Pixel clock from DSI PHY1
>>>>> +      - description: Link clock from DP PHY
>>>>> +      - description: VCO DIV clock from DP PHY
>>>>> +      - description: AHB config clock from GCC
>>>>> +
>>>>> +  clock-names:
>>>>> +    items:
>>>>> +      - const: bi_tcxo
>>>>> +      - const: dsi0_phy_pll_out_byteclk
>>>>> +      - const: dsi0_phy_pll_out_dsiclk
>>>>> +      - const: dsi1_phy_pll_out_dsiclk
>>>>> +      - const: dp_phy_pll_link_clk
>>>>> +      - const: dp_phy_pll_vco_div_clk
>>>>> +      - const: cfg_ahb_clk
>>>>> +
>>>>> +  '#clock-cells':
>>>>> +    const: 1
>>>>> +
>>>>> +  '#power-domain-cells':
>>>>> +    const: 1
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - reg
>>>>> +  - clocks
>>>>> +  - clock-names
>>>>> +  - '#clock-cells'
>>>>> +  - '#power-domain-cells'
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    #include <dt-bindings/clock/qcom,rpmcc.h>
>>>>> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>>>>> +    clock-controller@5f00000 {
>>>>> +      compatible = "qcom,sm6125-dispcc";
>>>>> +      reg = <0x5f00000 0x20000>;
>>>>> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>>>>> +               <&dsi0_phy 0>,
>>>>> +               <&dsi0_phy 1>,
>>>>> +               <0>,
>>>>
>>>> This does not look like a valid phandle. This clock is required, isn't it?
> 
> I remember it being used like this before, though upon closer inspection
> only qcom,gcc-msm8998.yaml uses it as example.
> 
> The clock should be optional, in that case it is perhaps desired to omit
> it from clock-names instead, or pretend there's a `dsi1_phy 1`?

I propose to omit it.

> 
>>>
>>> Not, it's not required for general dispcc support.
>>> dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc 
>>> clocks. However if support for DP is not enabled, the dispcc can work 
>>> w/o DP phy clock. Thus we typically add 0 phandles as placeholders for 
> 
> Is there any semantic difference between omitting the clock from DT (in
> clocks= /and/ clock-names=) or setting it to a 0 phandle?

Yes, there is. The DT validation does not check the meaning behind
values, so there is no difference between valid phandle/ID and 0. While
not having a clock at all is spotted by validation.

> 
>>> DSI/DP clock sources and populate them as support for respective 
>>> interfaces gets implemented.
>>>
>>
>> Then the clock is optional, isn't it? While not modeling it as optional?
> 
> It looks like this should be modelled using minItems: then, and
> "optional" text/comment? Other clocks are optional as well, we don't
> have DSI 1 in downstream SM6125 DT sources and haven't added the DP PLL
> in our to-be-upstreamed mainline tree yet.

Are they really optional? Or maybe they should not even be provided?


Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Bjorn Andersson]

On Wed 02 Mar 05:51 PST 2022, Krzysztof Kozlowski wrote:

> On 02/03/2022 13:54, Marijn Suijten wrote:
> > On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
> >> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
> >>> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
> >>>> On 26/02/2022 21:09, Marijn Suijten wrote:
> >>>>> From: Martin Botka <martin.botka@somainline.org>
> >>>>>
> >>>>> Add device tree bindings for display clock controller for
> >>>>> Qualcomm Technology Inc's SM6125 SoC.
> >>>>>
> >>>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> >>>>> ---
> >>>>>   .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
> >>>>>   .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
> >>>>>   2 files changed, 128 insertions(+)
> >>>>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>>>>   create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..3465042d0d9f
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>>>> @@ -0,0 +1,87 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: Qualcomm Display Clock Controller Binding for SM6125
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Martin Botka <martin.botka@somainline.org>
> >>>>> +
> >>>>> +description: |
> >>>>> +  Qualcomm display clock control module which supports the clocks and
> >>>>> +  power domains on SM6125.
> >>>>> +
> >>>>> +  See also:
> >>>>> +    dt-bindings/clock/qcom,dispcc-sm6125.h
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    enum:
> >>>>> +      - qcom,sm6125-dispcc
> >>>>> +
> >>>>> +  clocks:
> >>>>> +    items:
> >>>>> +      - description: Board XO source
> >>>>> +      - description: Byte clock from DSI PHY0
> >>>>> +      - description: Pixel clock from DSI PHY0
> >>>>> +      - description: Pixel clock from DSI PHY1
> >>>>> +      - description: Link clock from DP PHY
> >>>>> +      - description: VCO DIV clock from DP PHY
> >>>>> +      - description: AHB config clock from GCC
> >>>>> +
> >>>>> +  clock-names:
> >>>>> +    items:
> >>>>> +      - const: bi_tcxo
> >>>>> +      - const: dsi0_phy_pll_out_byteclk
> >>>>> +      - const: dsi0_phy_pll_out_dsiclk
> >>>>> +      - const: dsi1_phy_pll_out_dsiclk
> >>>>> +      - const: dp_phy_pll_link_clk
> >>>>> +      - const: dp_phy_pll_vco_div_clk
> >>>>> +      - const: cfg_ahb_clk
> >>>>> +
> >>>>> +  '#clock-cells':
> >>>>> +    const: 1
> >>>>> +
> >>>>> +  '#power-domain-cells':
> >>>>> +    const: 1
> >>>>> +
> >>>>> +  reg:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +required:
> >>>>> +  - compatible
> >>>>> +  - reg
> >>>>> +  - clocks
> >>>>> +  - clock-names
> >>>>> +  - '#clock-cells'
> >>>>> +  - '#power-domain-cells'
> >>>>> +
> >>>>> +additionalProperties: false
> >>>>> +
> >>>>> +examples:
> >>>>> +  - |
> >>>>> +    #include <dt-bindings/clock/qcom,rpmcc.h>
> >>>>> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> >>>>> +    clock-controller@5f00000 {
> >>>>> +      compatible = "qcom,sm6125-dispcc";
> >>>>> +      reg = <0x5f00000 0x20000>;
> >>>>> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> >>>>> +               <&dsi0_phy 0>,
> >>>>> +               <&dsi0_phy 1>,
> >>>>> +               <0>,
> >>>>
> >>>> This does not look like a valid phandle. This clock is required, isn't it?
> > 
> > I remember it being used like this before, though upon closer inspection
> > only qcom,gcc-msm8998.yaml uses it as example.
> > 
> > The clock should be optional, in that case it is perhaps desired to omit
> > it from clock-names instead, or pretend there's a `dsi1_phy 1`?
> 
> I propose to omit it.
> 

The wire is there, it's only optional because we don't have the other
side represented in DT yet.

I believe we started filling out 0s like this because omitting elements
that are not yet possible to fill out means that the order will change
as we add more functions, something Rob has objected to. Further more as
we add more functions the existing dts will fail validation, even though
the hardware hasn't changed.


That said, even though we don't have the other piece on this particular
platform we do know where this signal comes from. So we should be able
to have a valid (or at least strongly plausible) example in the binding
- and then fill out the dts with 0s to keep validation happy until the
other pieces are filled out.

Regards,
Bjorn

> > 
> >>>
> >>> Not, it's not required for general dispcc support.
> >>> dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc 
> >>> clocks. However if support for DP is not enabled, the dispcc can work 
> >>> w/o DP phy clock. Thus we typically add 0 phandles as placeholders for 
> > 
> > Is there any semantic difference between omitting the clock from DT (in
> > clocks= /and/ clock-names=) or setting it to a 0 phandle?
> 
> Yes, there is. The DT validation does not check the meaning behind
> values, so there is no difference between valid phandle/ID and 0. While
> not having a clock at all is spotted by validation.
> 
> > 
> >>> DSI/DP clock sources and populate them as support for respective 
> >>> interfaces gets implemented.
> >>>
> >>
> >> Then the clock is optional, isn't it? While not modeling it as optional?
> > 
> > It looks like this should be modelled using minItems: then, and
> > "optional" text/comment? Other clocks are optional as well, we don't
> > have DSI 1 in downstream SM6125 DT sources and haven't added the DP PLL
> > in our to-be-upstreamed mainline tree yet.
> 
> Are they really optional? Or maybe they should not even be provided?
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings

[Krzysztof Kozlowski]

On 02/03/2022 15:48, Bjorn Andersson wrote:
> On Wed 02 Mar 05:51 PST 2022, Krzysztof Kozlowski wrote:
> 
>> On 02/03/2022 13:54, Marijn Suijten wrote:
>>> On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
>>>> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
>>>>> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
>>>>>> On 26/02/2022 21:09, Marijn Suijten wrote:
>>>>>>> From: Martin Botka <martin.botka@somainline.org>
>>>>>>>
>>>>>>> Add device tree bindings for display clock controller for
>>>>>>> Qualcomm Technology Inc's SM6125 SoC.
>>>>>>>
>>>>>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>>>>>> ---
>>>>>>>   .../bindings/clock/qcom,dispcc-sm6125.yaml    | 87 +++++++++++++++++++
>>>>>>>   .../dt-bindings/clock/qcom,dispcc-sm6125.h    | 41 +++++++++
>>>>>>>   2 files changed, 128 insertions(+)
>>>>>>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>>>>   create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..3465042d0d9f
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>>>> @@ -0,0 +1,87 @@
>>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>>> +%YAML 1.2
>>>>>>> +---
>>>>>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>> +
>>>>>>> +title: Qualcomm Display Clock Controller Binding for SM6125
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> +  - Martin Botka <martin.botka@somainline.org>
>>>>>>> +
>>>>>>> +description: |
>>>>>>> +  Qualcomm display clock control module which supports the clocks and
>>>>>>> +  power domains on SM6125.
>>>>>>> +
>>>>>>> +  See also:
>>>>>>> +    dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>>>> +
>>>>>>> +properties:
>>>>>>> +  compatible:
>>>>>>> +    enum:
>>>>>>> +      - qcom,sm6125-dispcc
>>>>>>> +
>>>>>>> +  clocks:
>>>>>>> +    items:
>>>>>>> +      - description: Board XO source
>>>>>>> +      - description: Byte clock from DSI PHY0
>>>>>>> +      - description: Pixel clock from DSI PHY0
>>>>>>> +      - description: Pixel clock from DSI PHY1
>>>>>>> +      - description: Link clock from DP PHY
>>>>>>> +      - description: VCO DIV clock from DP PHY
>>>>>>> +      - description: AHB config clock from GCC
>>>>>>> +
>>>>>>> +  clock-names:
>>>>>>> +    items:
>>>>>>> +      - const: bi_tcxo
>>>>>>> +      - const: dsi0_phy_pll_out_byteclk
>>>>>>> +      - const: dsi0_phy_pll_out_dsiclk
>>>>>>> +      - const: dsi1_phy_pll_out_dsiclk
>>>>>>> +      - const: dp_phy_pll_link_clk
>>>>>>> +      - const: dp_phy_pll_vco_div_clk
>>>>>>> +      - const: cfg_ahb_clk
>>>>>>> +
>>>>>>> +  '#clock-cells':
>>>>>>> +    const: 1
>>>>>>> +
>>>>>>> +  '#power-domain-cells':
>>>>>>> +    const: 1
>>>>>>> +
>>>>>>> +  reg:
>>>>>>> +    maxItems: 1
>>>>>>> +
>>>>>>> +required:
>>>>>>> +  - compatible
>>>>>>> +  - reg
>>>>>>> +  - clocks
>>>>>>> +  - clock-names
>>>>>>> +  - '#clock-cells'
>>>>>>> +  - '#power-domain-cells'
>>>>>>> +
>>>>>>> +additionalProperties: false
>>>>>>> +
>>>>>>> +examples:
>>>>>>> +  - |
>>>>>>> +    #include <dt-bindings/clock/qcom,rpmcc.h>
>>>>>>> +    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>>>>>>> +    clock-controller@5f00000 {
>>>>>>> +      compatible = "qcom,sm6125-dispcc";
>>>>>>> +      reg = <0x5f00000 0x20000>;
>>>>>>> +      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>>>>>>> +               <&dsi0_phy 0>,
>>>>>>> +               <&dsi0_phy 1>,
>>>>>>> +               <0>,
>>>>>>
>>>>>> This does not look like a valid phandle. This clock is required, isn't it?
>>>
>>> I remember it being used like this before, though upon closer inspection
>>> only qcom,gcc-msm8998.yaml uses it as example.
>>>
>>> The clock should be optional, in that case it is perhaps desired to omit
>>> it from clock-names instead, or pretend there's a `dsi1_phy 1`?
>>
>> I propose to omit it.
>>
> 
> The wire is there, it's only optional because we don't have the other
> side represented in DT yet.
> 
> I believe we started filling out 0s like this because omitting elements
> that are not yet possible to fill out means that the order will change
> as we add more functions, something Rob has objected to. Further more as
> we add more functions the existing dts will fail validation, even though
> the hardware hasn't changed.
> 
> 
> That said, even though we don't have the other piece on this particular
> platform we do know where this signal comes from. So we should be able
> to have a valid (or at least strongly plausible) example in the binding
> - and then fill out the dts with 0s to keep validation happy until the
> other pieces are filled out.

So based on this, this clock is not actually optional and the bindings
should stay like this. The example should be more-or-less complete, so
there is not much sense to have there clock "0". DTS is of course different.

BR,
Krzysztof