Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc

All of lore.kernel.org
 help / color / mirror / Atom feed

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
@ 2024-02-09 10:39 kernel test robot
  0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-02-09 10:39 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20240206113145.31096-3-quic_jkona@quicinc.com>
References: <20240206113145.31096-3-quic_jkona@quicinc.com>
TO: Jagadeesh Kona <quic_jkona@quicinc.com>
TO: Bjorn Andersson <andersson@kernel.org>
TO: Konrad Dybcio <konrad.dybcio@linaro.org>
TO: Michael Turquette <mturquette@baylibre.com>
TO: Stephen Boyd <sboyd@kernel.org>
TO: Rob Herring <robh@kernel.org>
TO: Krzysztof Kozlowski <krzk@kernel.org>
TO: Conor Dooley <conor+dt@kernel.org>
CC: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
CC: Taniya Das <quic_tdas@quicinc.com>
CC: linux-arm-msm@vger.kernel.org
CC: linux-clk@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Imran Shaik <quic_imrashai@quicinc.com>
CC: Ajit Pandey <quic_ajipan@quicinc.com>
CC: Jagadeesh Kona <quic_jkona@quicinc.com>

Hi Jagadeesh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on robh/for-next linus/master v6.8-rc3]
[cannot apply to next-20240209]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jagadeesh-Kona/dt-bindings-clock-qcom-Add-video-clock-bindings-for-SM8650/20240206-194148
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/20240206113145.31096-3-quic_jkona%40quicinc.com
patch subject: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: arm64-randconfig-r071-20240207 (https://download.01.org/0day-ci/archive/20240209/202402091804.SdrSLt10-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202402091804.SdrSLt10-lkp@intel.com/

smatch warnings:
drivers/clk/qcom/videocc-sm8550.c:590 video_cc_sm8550_probe() error: uninitialized symbol 'offset'.

vim +/offset +590 drivers/clk/qcom/videocc-sm8550.c

f53153a37969c1 Jagadeesh Kona   2023-05-24  542  
f53153a37969c1 Jagadeesh Kona   2023-05-24  543  static int video_cc_sm8550_probe(struct platform_device *pdev)
f53153a37969c1 Jagadeesh Kona   2023-05-24  544  {
f53153a37969c1 Jagadeesh Kona   2023-05-24  545  	struct regmap *regmap;
f53153a37969c1 Jagadeesh Kona   2023-05-24  546  	int ret;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  547  	u32 offset;
f53153a37969c1 Jagadeesh Kona   2023-05-24  548  
f53153a37969c1 Jagadeesh Kona   2023-05-24  549  	ret = devm_pm_runtime_enable(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  550  	if (ret)
f53153a37969c1 Jagadeesh Kona   2023-05-24  551  		return ret;
f53153a37969c1 Jagadeesh Kona   2023-05-24  552  
f53153a37969c1 Jagadeesh Kona   2023-05-24  553  	ret = pm_runtime_resume_and_get(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  554  	if (ret)
f53153a37969c1 Jagadeesh Kona   2023-05-24  555  		return ret;
f53153a37969c1 Jagadeesh Kona   2023-05-24  556  
f53153a37969c1 Jagadeesh Kona   2023-05-24  557  	regmap = qcom_cc_map(pdev, &video_cc_sm8550_desc);
f53153a37969c1 Jagadeesh Kona   2023-05-24  558  	if (IS_ERR(regmap)) {
f53153a37969c1 Jagadeesh Kona   2023-05-24  559  		pm_runtime_put(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  560  		return PTR_ERR(regmap);
f53153a37969c1 Jagadeesh Kona   2023-05-24  561  	}
f53153a37969c1 Jagadeesh Kona   2023-05-24  562  
5ab3df7257a04f Jagadeesh Kona   2024-02-06  563  	if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
5ab3df7257a04f Jagadeesh Kona   2024-02-06  564  		video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  565  		video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  566  		video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  567  		video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  568  		video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  569  		offset = 0x8140;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  570  	} else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
5ab3df7257a04f Jagadeesh Kona   2024-02-06  571  		video_cc_pll0_config.l = 0x1e;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  572  		video_cc_pll0_config.alpha = 0xa000;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  573  		video_cc_pll1_config.l = 0x2b;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  574  		video_cc_pll1_config.alpha = 0xc000;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  575  		video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  576  		video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  577  		offset = 0x8150;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  578  	}
5ab3df7257a04f Jagadeesh Kona   2024-02-06  579  
a2620539ae2529 Dmitry Baryshkov 2023-10-16  580  	clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
a2620539ae2529 Dmitry Baryshkov 2023-10-16  581  	clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
f53153a37969c1 Jagadeesh Kona   2023-05-24  582  
f53153a37969c1 Jagadeesh Kona   2023-05-24  583  	/*
f53153a37969c1 Jagadeesh Kona   2023-05-24  584  	 * Keep clocks always enabled:
f53153a37969c1 Jagadeesh Kona   2023-05-24  585  	 *	video_cc_ahb_clk
f53153a37969c1 Jagadeesh Kona   2023-05-24  586  	 *	video_cc_sleep_clk
f53153a37969c1 Jagadeesh Kona   2023-05-24  587  	 *	video_cc_xo_clk
f53153a37969c1 Jagadeesh Kona   2023-05-24  588  	 */
f53153a37969c1 Jagadeesh Kona   2023-05-24  589  	regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
5ab3df7257a04f Jagadeesh Kona   2024-02-06 @590  	regmap_update_bits(regmap, offset, BIT(0), BIT(0));
f53153a37969c1 Jagadeesh Kona   2023-05-24  591  	regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
f53153a37969c1 Jagadeesh Kona   2023-05-24  592  
f53153a37969c1 Jagadeesh Kona   2023-05-24  593  	ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
f53153a37969c1 Jagadeesh Kona   2023-05-24  594  
f53153a37969c1 Jagadeesh Kona   2023-05-24  595  	pm_runtime_put(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  596  
f53153a37969c1 Jagadeesh Kona   2023-05-24  597  	return ret;
f53153a37969c1 Jagadeesh Kona   2023-05-24  598  }
f53153a37969c1 Jagadeesh Kona   2023-05-24  599  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* [PATCH 0/5] Add support for videocc and camcc on SM8650
@ 2024-02-06 11:31 Jagadeesh Kona
  2024-02-06 11:31 ` [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc Jagadeesh Kona
  0 siblings, 1 reply; 10+ messages in thread
From: Jagadeesh Kona @ 2024-02-06 11:31 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey,
	Jagadeesh Kona

Add support for video and camera clock controllers on Qualcomm
SM8650 platform.

Jagadeesh Kona (5):
  dt-bindings: clock: qcom: Add video clock bindings for SM8650
  clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  dt-bindings: clock: qcom: Add SM8650 camera clock controller
  clk: qcom: camcc-sm8650: Add camera clock controller driver for SM8650
  arm64: dts: qcom: sm8650: Add video and camera clock controllers

 .../bindings/clock/qcom,sm8450-camcc.yaml     |    3 +
 .../bindings/clock/qcom,sm8450-videocc.yaml   |    4 +-
 arch/arm64/boot/dts/qcom/sm8650.dtsi          |   28 +
 drivers/clk/qcom/Kconfig                      |    8 +
 drivers/clk/qcom/Makefile                     |    1 +
 drivers/clk/qcom/camcc-sm8650.c               | 3601 +++++++++++++++++
 drivers/clk/qcom/videocc-sm8550.c             |  160 +-
 .../dt-bindings/clock/qcom,sm8450-videocc.h   |    8 +-
 include/dt-bindings/clock/qcom,sm8650-camcc.h |  195 +
 9 files changed, 4002 insertions(+), 6 deletions(-)
 create mode 100644 drivers/clk/qcom/camcc-sm8650.c
 create mode 100644 include/dt-bindings/clock/qcom,sm8650-camcc.h

-- 
2.43.0


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

* [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-06 11:31 [PATCH 0/5] Add support for videocc and camcc on SM8650 Jagadeesh Kona
@ 2024-02-06 11:31 ` Jagadeesh Kona
  2024-02-06 11:54   ` Dmitry Baryshkov
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jagadeesh Kona @ 2024-02-06 11:31 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey,
	Jagadeesh Kona

Add support to the SM8650 video clock controller by extending the
SM8550 video clock controller, which is mostly identical but SM8650
has few additional clocks and minor differences.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/videocc-sm8550.c | 160 +++++++++++++++++++++++++++++-
 1 file changed, 156 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
index f3c9dfaee968..cdc08f5900fc 100644
--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #include <linux/clk-provider.h>
@@ -35,7 +35,7 @@ static const struct pll_vco lucid_ole_vco[] = {
 	{ 249600000, 2300000000, 0 },
 };
 
-static const struct alpha_pll_config video_cc_pll0_config = {
+static struct alpha_pll_config video_cc_pll0_config = {
 	.l = 0x25,
 	.alpha = 0x8000,
 	.config_ctl_val = 0x20485699,
@@ -66,7 +66,7 @@ static struct clk_alpha_pll video_cc_pll0 = {
 	},
 };
 
-static const struct alpha_pll_config video_cc_pll1_config = {
+static struct alpha_pll_config video_cc_pll1_config = {
 	.l = 0x36,
 	.alpha = 0xb000,
 	.config_ctl_val = 0x20485699,
@@ -117,6 +117,14 @@ static const struct clk_parent_data video_cc_parent_data_1[] = {
 	{ .hw = &video_cc_pll1.clkr.hw },
 };
 
+static const struct parent_map video_cc_parent_map_2[] = {
+	{ P_BI_TCXO, 0 },
+};
+
+static const struct clk_parent_data video_cc_parent_data_2[] = {
+	{ .index = DT_BI_TCXO },
+};
+
 static const struct freq_tbl ftbl_video_cc_mvs0_clk_src[] = {
 	F(720000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
 	F(1014000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
@@ -126,6 +134,16 @@ static const struct freq_tbl ftbl_video_cc_mvs0_clk_src[] = {
 	{ }
 };
 
+static const struct freq_tbl ftbl_video_cc_mvs0_clk_src_sm8650[] = {
+	F(588000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
+	F(900000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
+	F(1140000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
+	F(1305000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
+	F(1440000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
+	F(1600000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
+	{ }
+};
+
 static struct clk_rcg2 video_cc_mvs0_clk_src = {
 	.cmd_rcgr = 0x8000,
 	.mnd_width = 0,
@@ -149,6 +167,15 @@ static const struct freq_tbl ftbl_video_cc_mvs1_clk_src[] = {
 	{ }
 };
 
+static const struct freq_tbl ftbl_video_cc_mvs1_clk_src_sm8650[] = {
+	F(840000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
+	F(1110000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
+	F(1350000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
+	F(1500000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
+	F(1650000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
+	{ }
+};
+
 static struct clk_rcg2 video_cc_mvs1_clk_src = {
 	.cmd_rcgr = 0x8018,
 	.mnd_width = 0,
@@ -164,6 +191,26 @@ static struct clk_rcg2 video_cc_mvs1_clk_src = {
 	},
 };
 
+static const struct freq_tbl ftbl_video_cc_xo_clk_src[] = {
+	F(19200000, P_BI_TCXO, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 video_cc_xo_clk_src = {
+	.cmd_rcgr = 0x810c,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = video_cc_parent_map_2,
+	.freq_tbl = ftbl_video_cc_xo_clk_src,
+	.clkr.hw.init = &(const struct clk_init_data) {
+		.name = "video_cc_xo_clk_src",
+		.parent_data = video_cc_parent_data_2,
+		.num_parents = ARRAY_SIZE(video_cc_parent_data_2),
+		.flags = CLK_SET_RATE_PARENT,
+		.ops = &clk_rcg2_shared_ops,
+	},
+};
+
 static struct clk_regmap_div video_cc_mvs0_div_clk_src = {
 	.reg = 0x80c4,
 	.shift = 0,
@@ -244,6 +291,26 @@ static struct clk_branch video_cc_mvs0_clk = {
 	},
 };
 
+static struct clk_branch video_cc_mvs0_shift_clk = {
+	.halt_reg = 0x8128,
+	.halt_check = BRANCH_HALT_VOTED,
+	.hwcg_reg = 0x8128,
+	.hwcg_bit = 1,
+	.clkr = {
+		.enable_reg = 0x8128,
+		.enable_mask = BIT(0),
+		.hw.init = &(const struct clk_init_data) {
+			.name = "video_cc_mvs0_shift_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&video_cc_xo_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch video_cc_mvs0c_clk = {
 	.halt_reg = 0x8064,
 	.halt_check = BRANCH_HALT,
@@ -262,6 +329,26 @@ static struct clk_branch video_cc_mvs0c_clk = {
 	},
 };
 
+static struct clk_branch video_cc_mvs0c_shift_clk = {
+	.halt_reg = 0x812c,
+	.halt_check = BRANCH_HALT_VOTED,
+	.hwcg_reg = 0x812c,
+	.hwcg_bit = 1,
+	.clkr = {
+		.enable_reg = 0x812c,
+		.enable_mask = BIT(0),
+		.hw.init = &(const struct clk_init_data) {
+			.name = "video_cc_mvs0c_shift_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&video_cc_xo_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch video_cc_mvs1_clk = {
 	.halt_reg = 0x80e0,
 	.halt_check = BRANCH_HALT_SKIP,
@@ -282,6 +369,26 @@ static struct clk_branch video_cc_mvs1_clk = {
 	},
 };
 
+static struct clk_branch video_cc_mvs1_shift_clk = {
+	.halt_reg = 0x8130,
+	.halt_check = BRANCH_HALT_VOTED,
+	.hwcg_reg = 0x8130,
+	.hwcg_bit = 1,
+	.clkr = {
+		.enable_reg = 0x8130,
+		.enable_mask = BIT(0),
+		.hw.init = &(const struct clk_init_data) {
+			.name = "video_cc_mvs1_shift_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&video_cc_xo_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch video_cc_mvs1c_clk = {
 	.halt_reg = 0x8090,
 	.halt_check = BRANCH_HALT,
@@ -300,6 +407,26 @@ static struct clk_branch video_cc_mvs1c_clk = {
 	},
 };
 
+static struct clk_branch video_cc_mvs1c_shift_clk = {
+	.halt_reg = 0x8134,
+	.halt_check = BRANCH_HALT_VOTED,
+	.hwcg_reg = 0x8134,
+	.hwcg_bit = 1,
+	.clkr = {
+		.enable_reg = 0x8134,
+		.enable_mask = BIT(0),
+		.hw.init = &(const struct clk_init_data) {
+			.name = "video_cc_mvs1c_shift_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&video_cc_xo_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct gdsc video_cc_mvs0c_gdsc = {
 	.gdscr = 0x804c,
 	.en_rest_wait_val = 0x2,
@@ -354,15 +481,20 @@ static struct clk_regmap *video_cc_sm8550_clocks[] = {
 	[VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
 	[VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
 	[VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
+	[VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
 	[VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
 	[VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
+	[VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
 	[VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
 	[VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
 	[VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
+	[VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
 	[VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
 	[VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
+	[VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
 	[VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
 	[VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
+	[VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
 };
 
 static struct gdsc *video_cc_sm8550_gdscs[] = {
@@ -380,6 +512,7 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
 	[CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
 	[VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
 	[VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
+	[VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },
 };
 
 static const struct regmap_config video_cc_sm8550_regmap_config = {
@@ -402,6 +535,7 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
 
 static const struct of_device_id video_cc_sm8550_match_table[] = {
 	{ .compatible = "qcom,sm8550-videocc" },
+	{ .compatible = "qcom,sm8650-videocc" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
@@ -410,6 +544,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
 	int ret;
+	u32 offset;
 
 	ret = devm_pm_runtime_enable(&pdev->dev);
 	if (ret)
@@ -425,6 +560,23 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
 		return PTR_ERR(regmap);
 	}
 
+	if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
+		video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
+		video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
+		video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
+		video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
+		video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
+		offset = 0x8140;
+	} else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
+		video_cc_pll0_config.l = 0x1e;
+		video_cc_pll0_config.alpha = 0xa000;
+		video_cc_pll1_config.l = 0x2b;
+		video_cc_pll1_config.alpha = 0xc000;
+		video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
+		video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
+		offset = 0x8150;
+	}
+
 	clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
 	clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
 
@@ -435,7 +587,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
 	 *	video_cc_xo_clk
 	 */
 	regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
-	regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
+	regmap_update_bits(regmap, offset, BIT(0), BIT(0));
 	regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
 
 	ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
-- 
2.43.0


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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-06 11:31 ` [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc Jagadeesh Kona
@ 2024-02-06 11:54   ` Dmitry Baryshkov
  2024-02-07  6:58     ` Jagadeesh Kona
  2024-02-07  3:49   ` kernel test robot
  2024-02-09 10:44   ` Dan Carpenter
  2 siblings, 1 reply; 10+ messages in thread
From: Dmitry Baryshkov @ 2024-02-06 11:54 UTC (permalink / raw)
  To: Jagadeesh Kona
  Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey

On Tue, 6 Feb 2024 at 13:39, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>
> Add support to the SM8650 video clock controller by extending the
> SM8550 video clock controller, which is mostly identical but SM8650
> has few additional clocks and minor differences.

In the past we tried merging similar clock controllers. In the end
this results in the ugly source code. Please consider submitting a
separate driver.

>
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  drivers/clk/qcom/videocc-sm8550.c | 160 +++++++++++++++++++++++++++++-
>  1 file changed, 156 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
> index f3c9dfaee968..cdc08f5900fc 100644
> --- a/drivers/clk/qcom/videocc-sm8550.c
> +++ b/drivers/clk/qcom/videocc-sm8550.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  #include <linux/clk-provider.h>

[skipping]

>  static struct gdsc video_cc_mvs0c_gdsc = {
>         .gdscr = 0x804c,
>         .en_rest_wait_val = 0x2,
> @@ -354,15 +481,20 @@ static struct clk_regmap *video_cc_sm8550_clocks[] = {
>         [VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
>         [VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
>         [VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
> +       [VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
>         [VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
>         [VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
> +       [VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
>         [VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
>         [VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
>         [VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
> +       [VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
>         [VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
>         [VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
> +       [VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
>         [VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
>         [VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
> +       [VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
>  };
>
>  static struct gdsc *video_cc_sm8550_gdscs[] = {
> @@ -380,6 +512,7 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
>         [CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
>         [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
>         [VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
> +       [VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },

Is this reset applicable to videocc-sm8550?

>  };
>
>  static const struct regmap_config video_cc_sm8550_regmap_config = {
> @@ -402,6 +535,7 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
>
>  static const struct of_device_id video_cc_sm8550_match_table[] = {
>         { .compatible = "qcom,sm8550-videocc" },
> +       { .compatible = "qcom,sm8650-videocc" },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
> @@ -410,6 +544,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>  {
>         struct regmap *regmap;
>         int ret;
> +       u32 offset;
>
>         ret = devm_pm_runtime_enable(&pdev->dev);
>         if (ret)
> @@ -425,6 +560,23 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>                 return PTR_ERR(regmap);
>         }
>
> +       if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
> +               video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;

Please invert the logic. Make video_cc_sm8550_clocks reflect SM8550
and patch in new clocks in the SM8650-specific branch below.

> +               offset = 0x8140;
> +       } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
> +               video_cc_pll0_config.l = 0x1e;
> +               video_cc_pll0_config.alpha = 0xa000;
> +               video_cc_pll1_config.l = 0x2b;
> +               video_cc_pll1_config.alpha = 0xc000;
> +               video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
> +               video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
> +               offset = 0x8150;
> +       }
> +
>         clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
>         clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
>
> @@ -435,7 +587,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>          *      video_cc_xo_clk
>          */
>         regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
> -       regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
> +       regmap_update_bits(regmap, offset, BIT(0), BIT(0));
>         regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
>
>         ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
> --
> 2.43.0
>
>


-- 
With best wishes
Dmitry

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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-06 11:54   ` Dmitry Baryshkov
@ 2024-02-07  6:58     ` Jagadeesh Kona
  2024-02-07  7:19       ` Dmitry Baryshkov
  0 siblings, 1 reply; 10+ messages in thread
From: Jagadeesh Kona @ 2024-02-07  6:58 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey



On 2/6/2024 5:24 PM, Dmitry Baryshkov wrote:
> On Tue, 6 Feb 2024 at 13:39, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>> Add support to the SM8650 video clock controller by extending the
>> SM8550 video clock controller, which is mostly identical but SM8650
>> has few additional clocks and minor differences.
> 
> In the past we tried merging similar clock controllers. In the end
> this results in the ugly source code. Please consider submitting a
> separate driver.
> 

Thanks Dmitry for your review. SM8650 has only few clock additions and 
minor changes compared to SM8550, so I believe it is better to reuse 
this existing driver and extend it.

>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>>   drivers/clk/qcom/videocc-sm8550.c | 160 +++++++++++++++++++++++++++++-
>>   1 file changed, 156 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
>> index f3c9dfaee968..cdc08f5900fc 100644
>> --- a/drivers/clk/qcom/videocc-sm8550.c
>> +++ b/drivers/clk/qcom/videocc-sm8550.c
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>> - * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   #include <linux/clk-provider.h>
> 
> [skipping]
> 
>>   static struct gdsc video_cc_mvs0c_gdsc = {
>>          .gdscr = 0x804c,
>>          .en_rest_wait_val = 0x2,
>> @@ -354,15 +481,20 @@ static struct clk_regmap *video_cc_sm8550_clocks[] = {
>>          [VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
>>          [VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
>>          [VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
>> +       [VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
>>          [VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
>>          [VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
>> +       [VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
>>          [VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
>>          [VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
>>          [VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
>> +       [VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
>>          [VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
>>          [VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
>> +       [VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
>>          [VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
>>          [VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
>> +       [VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
>>   };
>>
>>   static struct gdsc *video_cc_sm8550_gdscs[] = {
>> @@ -380,6 +512,7 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
>>          [CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
>>          [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
>>          [VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
>> +       [VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },
> 
> Is this reset applicable to videocc-sm8550?
> 

SM8550 also has above reset support in hardware, hence it is safe to 
model above reset for both SM8550 and SM8650.

>>   };
>>
>>   static const struct regmap_config video_cc_sm8550_regmap_config = {
>> @@ -402,6 +535,7 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
>>
>>   static const struct of_device_id video_cc_sm8550_match_table[] = {
>>          { .compatible = "qcom,sm8550-videocc" },
>> +       { .compatible = "qcom,sm8650-videocc" },
>>          { }
>>   };
>>   MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
>> @@ -410,6 +544,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>   {
>>          struct regmap *regmap;
>>          int ret;
>> +       u32 offset;
>>
>>          ret = devm_pm_runtime_enable(&pdev->dev);
>>          if (ret)
>> @@ -425,6 +560,23 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>                  return PTR_ERR(regmap);
>>          }
>>
>> +       if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
>> +               video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
> 
> Please invert the logic. Make video_cc_sm8550_clocks reflect SM8550
> and patch in new clocks in the SM8650-specific branch below.
> 

Sure, will add these clocks as NULL in video_cc_sm8550_clocks and patch 
in new clocks here for SM8650. Then we can remove above check for SM8550.

Thanks,
Jagadeesh

>> +               offset = 0x8140;
>> +       } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
>> +               video_cc_pll0_config.l = 0x1e;
>> +               video_cc_pll0_config.alpha = 0xa000;
>> +               video_cc_pll1_config.l = 0x2b;
>> +               video_cc_pll1_config.alpha = 0xc000;
>> +               video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
>> +               video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
>> +               offset = 0x8150;
>> +       }
>> +
>>          clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
>>          clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
>>
>> @@ -435,7 +587,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>           *      video_cc_xo_clk
>>           */
>>          regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
>> -       regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
>> +       regmap_update_bits(regmap, offset, BIT(0), BIT(0));
>>          regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
>>
>>          ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
>> --
>> 2.43.0
>>
>>
> 
> 

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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-07  6:58     ` Jagadeesh Kona
@ 2024-02-07  7:19       ` Dmitry Baryshkov
  2024-02-12 13:06         ` Jagadeesh Kona
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Baryshkov @ 2024-02-07  7:19 UTC (permalink / raw)
  To: Jagadeesh Kona
  Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey

On Wed, 7 Feb 2024 at 08:59, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>
>
>
> On 2/6/2024 5:24 PM, Dmitry Baryshkov wrote:
> > On Tue, 6 Feb 2024 at 13:39, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
> >>
> >> Add support to the SM8650 video clock controller by extending the
> >> SM8550 video clock controller, which is mostly identical but SM8650
> >> has few additional clocks and minor differences.
> >
> > In the past we tried merging similar clock controllers. In the end
> > this results in the ugly source code. Please consider submitting a
> > separate driver.
> >
>
> Thanks Dmitry for your review. SM8650 has only few clock additions and
> minor changes compared to SM8550, so I believe it is better to reuse
> this existing driver and extend it.

I'd say, the final decision is on Bjorn and Konrad as maintainers.

>
> >>
> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >> ---
> >>   drivers/clk/qcom/videocc-sm8550.c | 160 +++++++++++++++++++++++++++++-
> >>   1 file changed, 156 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
> >> index f3c9dfaee968..cdc08f5900fc 100644
> >> --- a/drivers/clk/qcom/videocc-sm8550.c
> >> +++ b/drivers/clk/qcom/videocc-sm8550.c
> >> @@ -1,6 +1,6 @@
> >>   // SPDX-License-Identifier: GPL-2.0-only
> >>   /*
> >> - * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
> >> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
> >>    */
> >>
> >>   #include <linux/clk-provider.h>
> >
> > [skipping]
> >
> >>   static struct gdsc video_cc_mvs0c_gdsc = {
> >>          .gdscr = 0x804c,
> >>          .en_rest_wait_val = 0x2,
> >> @@ -354,15 +481,20 @@ static struct clk_regmap *video_cc_sm8550_clocks[] = {
> >>          [VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
> >>          [VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
> >>          [VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
> >> +       [VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
> >>          [VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
> >>          [VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
> >> +       [VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
> >>          [VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
> >>          [VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
> >>          [VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
> >> +       [VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
> >>          [VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
> >>          [VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
> >> +       [VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
> >>          [VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
> >>          [VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
> >> +       [VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
> >>   };
> >>
> >>   static struct gdsc *video_cc_sm8550_gdscs[] = {
> >> @@ -380,6 +512,7 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
> >>          [CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
> >>          [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
> >>          [VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
> >> +       [VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },
> >
> > Is this reset applicable to videocc-sm8550?
> >
>
> SM8550 also has above reset support in hardware, hence it is safe to
> model above reset for both SM8550 and SM8650.

Then, separate commit, Fixes tag.

>
> >>   };
> >>
> >>   static const struct regmap_config video_cc_sm8550_regmap_config = {
> >> @@ -402,6 +535,7 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
> >>
> >>   static const struct of_device_id video_cc_sm8550_match_table[] = {
> >>          { .compatible = "qcom,sm8550-videocc" },
> >> +       { .compatible = "qcom,sm8650-videocc" },
> >>          { }
> >>   };
> >>   MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
> >> @@ -410,6 +544,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >>   {
> >>          struct regmap *regmap;
> >>          int ret;
> >> +       u32 offset;
> >>
> >>          ret = devm_pm_runtime_enable(&pdev->dev);
> >>          if (ret)
> >> @@ -425,6 +560,23 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >>                  return PTR_ERR(regmap);
> >>          }
> >>
> >> +       if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
> >> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
> >> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
> >> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
> >> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
> >> +               video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
> >
> > Please invert the logic. Make video_cc_sm8550_clocks reflect SM8550
> > and patch in new clocks in the SM8650-specific branch below.
> >
>
> Sure, will add these clocks as NULL in video_cc_sm8550_clocks and patch
> in new clocks here for SM8650. Then we can remove above check for SM8550.

No need to set them to NULL, it is the default value. Just add them to
the sm8650 branch.

>
> Thanks,
> Jagadeesh
>
> >> +               offset = 0x8140;
> >> +       } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
> >> +               video_cc_pll0_config.l = 0x1e;
> >> +               video_cc_pll0_config.alpha = 0xa000;
> >> +               video_cc_pll1_config.l = 0x2b;
> >> +               video_cc_pll1_config.alpha = 0xc000;
> >> +               video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
> >> +               video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
> >> +               offset = 0x8150;
> >> +       }
> >> +
> >>          clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
> >>          clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
> >>
> >> @@ -435,7 +587,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >>           *      video_cc_xo_clk
> >>           */
> >>          regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
> >> -       regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
> >> +       regmap_update_bits(regmap, offset, BIT(0), BIT(0));
> >>          regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
> >>
> >>          ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
> >> --
> >> 2.43.0
> >>
> >>
> >
> >



-- 
With best wishes
Dmitry

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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-07  7:19       ` Dmitry Baryshkov
@ 2024-02-12 13:06         ` Jagadeesh Kona
  2024-02-12 13:18           ` Dmitry Baryshkov
  0 siblings, 1 reply; 10+ messages in thread
From: Jagadeesh Kona @ 2024-02-12 13:06 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey



On 2/7/2024 12:49 PM, Dmitry Baryshkov wrote:
> On Wed, 7 Feb 2024 at 08:59, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 2/6/2024 5:24 PM, Dmitry Baryshkov wrote:
>>> On Tue, 6 Feb 2024 at 13:39, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>
>>>> Add support to the SM8650 video clock controller by extending the
>>>> SM8550 video clock controller, which is mostly identical but SM8650
>>>> has few additional clocks and minor differences.
>>>
>>> In the past we tried merging similar clock controllers. In the end
>>> this results in the ugly source code. Please consider submitting a
>>> separate driver.
>>>
>>
>> Thanks Dmitry for your review. SM8650 has only few clock additions and
>> minor changes compared to SM8550, so I believe it is better to reuse
>> this existing driver and extend it.
> 
> I'd say, the final decision is on Bjorn and Konrad as maintainers.
> 
>>
>>>>
>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>> ---
>>>>    drivers/clk/qcom/videocc-sm8550.c | 160 +++++++++++++++++++++++++++++-
>>>>    1 file changed, 156 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
>>>> index f3c9dfaee968..cdc08f5900fc 100644
>>>> --- a/drivers/clk/qcom/videocc-sm8550.c
>>>> +++ b/drivers/clk/qcom/videocc-sm8550.c
>>>> @@ -1,6 +1,6 @@
>>>>    // SPDX-License-Identifier: GPL-2.0-only
>>>>    /*
>>>> - * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>>>> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>>>     */
>>>>
>>>>    #include <linux/clk-provider.h>
>>>
>>> [skipping]
>>>
>>>>    static struct gdsc video_cc_mvs0c_gdsc = {
>>>>           .gdscr = 0x804c,
>>>>           .en_rest_wait_val = 0x2,
>>>> @@ -354,15 +481,20 @@ static struct clk_regmap *video_cc_sm8550_clocks[] = {
>>>>           [VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
>>>>           [VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
>>>>           [VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
>>>> +       [VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
>>>>           [VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
>>>>           [VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
>>>> +       [VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
>>>>           [VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
>>>>           [VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
>>>>           [VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
>>>> +       [VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
>>>>           [VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
>>>>           [VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
>>>> +       [VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
>>>>           [VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
>>>>           [VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
>>>> +       [VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
>>>>    };
>>>>
>>>>    static struct gdsc *video_cc_sm8550_gdscs[] = {
>>>> @@ -380,6 +512,7 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
>>>>           [CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
>>>>           [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
>>>>           [VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
>>>> +       [VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },
>>>
>>> Is this reset applicable to videocc-sm8550?
>>>
>>
>> SM8550 also has above reset support in hardware, hence it is safe to
>> model above reset for both SM8550 and SM8650.
> 
> Then, separate commit, Fixes tag.
> 

Sure, will separate and add Fixes tag in next series.

>>
>>>>    };
>>>>
>>>>    static const struct regmap_config video_cc_sm8550_regmap_config = {
>>>> @@ -402,6 +535,7 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
>>>>
>>>>    static const struct of_device_id video_cc_sm8550_match_table[] = {
>>>>           { .compatible = "qcom,sm8550-videocc" },
>>>> +       { .compatible = "qcom,sm8650-videocc" },
>>>>           { }
>>>>    };
>>>>    MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
>>>> @@ -410,6 +544,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>>>    {
>>>>           struct regmap *regmap;
>>>>           int ret;
>>>> +       u32 offset;
>>>>
>>>>           ret = devm_pm_runtime_enable(&pdev->dev);
>>>>           if (ret)
>>>> @@ -425,6 +560,23 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>>>                   return PTR_ERR(regmap);
>>>>           }
>>>>
>>>> +       if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
>>>> +               video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
>>>
>>> Please invert the logic. Make video_cc_sm8550_clocks reflect SM8550
>>> and patch in new clocks in the SM8650-specific branch below.
>>>
>>
>> Sure, will add these clocks as NULL in video_cc_sm8550_clocks and patch
>> in new clocks here for SM8650. Then we can remove above check for SM8550.
> 
> No need to set them to NULL, it is the default value. Just add them to
> the sm8650 branch.
> 

The video_cc_sm8550_clocks[] array size is fixed and has memory 
allocated only for current sm8550 clocks. To be able to accommodate 
sm8650 clocks in the same array, we need to initialize the clocks to 
NULL as below snippet to increase the array size.

static struct clk_regmap *video_cc_sm8550_clocks[] = {
.....
	[VIDEO_CC_XO_CLK_SRC] = NULL,
}

Thanks,
Jagadeesh

>>
>> Thanks,
>> Jagadeesh
>>
>>>> +               offset = 0x8140;
>>>> +       } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
>>>> +               video_cc_pll0_config.l = 0x1e;
>>>> +               video_cc_pll0_config.alpha = 0xa000;
>>>> +               video_cc_pll1_config.l = 0x2b;
>>>> +               video_cc_pll1_config.alpha = 0xc000;
>>>> +               video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
>>>> +               video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
>>>> +               offset = 0x8150;
>>>> +       }
>>>> +
>>>>           clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
>>>>           clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
>>>>
>>>> @@ -435,7 +587,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>>>            *      video_cc_xo_clk
>>>>            */
>>>>           regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
>>>> -       regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
>>>> +       regmap_update_bits(regmap, offset, BIT(0), BIT(0));
>>>>           regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
>>>>
>>>>           ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
>>>> --
>>>> 2.43.0
>>>>
>>>>
>>>
>>>
> 
> 
> 

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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-12 13:06         ` Jagadeesh Kona
@ 2024-02-12 13:18           ` Dmitry Baryshkov
  2024-02-14  5:29             ` Jagadeesh Kona
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Baryshkov @ 2024-02-12 13:18 UTC (permalink / raw)
  To: Jagadeesh Kona
  Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey

On Mon, 12 Feb 2024 at 15:07, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>
>
>
> On 2/7/2024 12:49 PM, Dmitry Baryshkov wrote:
> > On Wed, 7 Feb 2024 at 08:59, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 2/6/2024 5:24 PM, Dmitry Baryshkov wrote:
> >>> On Tue, 6 Feb 2024 at 13:39, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
> >>>>
> >>>> Add support to the SM8650 video clock controller by extending the
> >>>> SM8550 video clock controller, which is mostly identical but SM8650
> >>>> has few additional clocks and minor differences.
> >>>
> >>> In the past we tried merging similar clock controllers. In the end
> >>> this results in the ugly source code. Please consider submitting a
> >>> separate driver.
> >>>
> >>
> >> Thanks Dmitry for your review. SM8650 has only few clock additions and
> >> minor changes compared to SM8550, so I believe it is better to reuse
> >> this existing driver and extend it.
> >
> > I'd say, the final decision is on Bjorn and Konrad as maintainers.
> >
> >>
> >>>>
> >>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >>>> ---
> >>>>    drivers/clk/qcom/videocc-sm8550.c | 160 +++++++++++++++++++++++++++++-
> >>>>    1 file changed, 156 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
> >>>> index f3c9dfaee968..cdc08f5900fc 100644
> >>>> --- a/drivers/clk/qcom/videocc-sm8550.c
> >>>> +++ b/drivers/clk/qcom/videocc-sm8550.c
> >>>> @@ -1,6 +1,6 @@
> >>>>    // SPDX-License-Identifier: GPL-2.0-only
> >>>>    /*
> >>>> - * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
> >>>> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
> >>>>     */
> >>>>
> >>>>    #include <linux/clk-provider.h>
> >>>
> >>> [skipping]
> >>>
> >>>>    static struct gdsc video_cc_mvs0c_gdsc = {
> >>>>           .gdscr = 0x804c,
> >>>>           .en_rest_wait_val = 0x2,
> >>>> @@ -354,15 +481,20 @@ static struct clk_regmap *video_cc_sm8550_clocks[] = {
> >>>>           [VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
> >>>>           [VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
> >>>>           [VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
> >>>> +       [VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
> >>>>           [VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
> >>>>           [VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
> >>>> +       [VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
> >>>>           [VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
> >>>>           [VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
> >>>>           [VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
> >>>> +       [VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
> >>>>           [VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
> >>>>           [VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
> >>>> +       [VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
> >>>>           [VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
> >>>>           [VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
> >>>> +       [VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
> >>>>    };
> >>>>
> >>>>    static struct gdsc *video_cc_sm8550_gdscs[] = {
> >>>> @@ -380,6 +512,7 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
> >>>>           [CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
> >>>>           [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
> >>>>           [VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
> >>>> +       [VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },
> >>>
> >>> Is this reset applicable to videocc-sm8550?
> >>>
> >>
> >> SM8550 also has above reset support in hardware, hence it is safe to
> >> model above reset for both SM8550 and SM8650.
> >
> > Then, separate commit, Fixes tag.
> >
>
> Sure, will separate and add Fixes tag in next series.
>
> >>
> >>>>    };
> >>>>
> >>>>    static const struct regmap_config video_cc_sm8550_regmap_config = {
> >>>> @@ -402,6 +535,7 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
> >>>>
> >>>>    static const struct of_device_id video_cc_sm8550_match_table[] = {
> >>>>           { .compatible = "qcom,sm8550-videocc" },
> >>>> +       { .compatible = "qcom,sm8650-videocc" },
> >>>>           { }
> >>>>    };
> >>>>    MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
> >>>> @@ -410,6 +544,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >>>>    {
> >>>>           struct regmap *regmap;
> >>>>           int ret;
> >>>> +       u32 offset;
> >>>>
> >>>>           ret = devm_pm_runtime_enable(&pdev->dev);
> >>>>           if (ret)
> >>>> @@ -425,6 +560,23 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >>>>                   return PTR_ERR(regmap);
> >>>>           }
> >>>>
> >>>> +       if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
> >>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
> >>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
> >>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
> >>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
> >>>> +               video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
> >>>
> >>> Please invert the logic. Make video_cc_sm8550_clocks reflect SM8550
> >>> and patch in new clocks in the SM8650-specific branch below.
> >>>
> >>
> >> Sure, will add these clocks as NULL in video_cc_sm8550_clocks and patch
> >> in new clocks here for SM8650. Then we can remove above check for SM8550.
> >
> > No need to set them to NULL, it is the default value. Just add them to
> > the sm8650 branch.
> >
>
> The video_cc_sm8550_clocks[] array size is fixed and has memory
> allocated only for current sm8550 clocks. To be able to accommodate
> sm8650 clocks in the same array, we need to initialize the clocks to
> NULL as below snippet to increase the array size.
>
> static struct clk_regmap *video_cc_sm8550_clocks[] = {
> .....
>         [VIDEO_CC_XO_CLK_SRC] = NULL,
> }

The question/comment was regarding video_cc_sm8550_probe() rather than
video_cc_sm8550_clocks.

>
> Thanks,
> Jagadeesh
>
> >>
> >> Thanks,
> >> Jagadeesh
> >>
> >>>> +               offset = 0x8140;
> >>>> +       } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
> >>>> +               video_cc_pll0_config.l = 0x1e;
> >>>> +               video_cc_pll0_config.alpha = 0xa000;
> >>>> +               video_cc_pll1_config.l = 0x2b;
> >>>> +               video_cc_pll1_config.alpha = 0xc000;
> >>>> +               video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
> >>>> +               video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
> >>>> +               offset = 0x8150;
> >>>> +       }
> >>>> +
> >>>>           clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
> >>>>           clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
> >>>>
> >>>> @@ -435,7 +587,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >>>>            *      video_cc_xo_clk
> >>>>            */
> >>>>           regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
> >>>> -       regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
> >>>> +       regmap_update_bits(regmap, offset, BIT(0), BIT(0));
> >>>>           regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
> >>>>
> >>>>           ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
> >>>> --
> >>>> 2.43.0
> >>>>
> >>>>
> >>>
> >>>
> >
> >
> >



-- 
With best wishes
Dmitry

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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-12 13:18           ` Dmitry Baryshkov
@ 2024-02-14  5:29             ` Jagadeesh Kona
  0 siblings, 0 replies; 10+ messages in thread
From: Jagadeesh Kona @ 2024-02-14  5:29 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Vladimir Zapolskiy, Taniya Das, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Imran Shaik, Ajit Pandey



On 2/12/2024 6:48 PM, Dmitry Baryshkov wrote:
> On Mon, 12 Feb 2024 at 15:07, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 2/7/2024 12:49 PM, Dmitry Baryshkov wrote:
>>> On Wed, 7 Feb 2024 at 08:59, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2/6/2024 5:24 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, 6 Feb 2024 at 13:39, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>>>
>>>>>> Add support to the SM8650 video clock controller by extending the
>>>>>> SM8550 video clock controller, which is mostly identical but SM8650
>>>>>> has few additional clocks and minor differences.
>>>>>
>>>>> In the past we tried merging similar clock controllers. In the end
>>>>> this results in the ugly source code. Please consider submitting a
>>>>> separate driver.
>>>>>
>>>>
>>>> Thanks Dmitry for your review. SM8650 has only few clock additions and
>>>> minor changes compared to SM8550, so I believe it is better to reuse
>>>> this existing driver and extend it.
>>>
>>> I'd say, the final decision is on Bjorn and Konrad as maintainers.
>>>
>>>>
>>>>>>
>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>> ---
>>>>>>     drivers/clk/qcom/videocc-sm8550.c | 160 +++++++++++++++++++++++++++++-
>>>>>>     1 file changed, 156 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
>>>>>> index f3c9dfaee968..cdc08f5900fc 100644
>>>>>> --- a/drivers/clk/qcom/videocc-sm8550.c
>>>>>> +++ b/drivers/clk/qcom/videocc-sm8550.c
>>>>>> @@ -1,6 +1,6 @@
>>>>>>     // SPDX-License-Identifier: GPL-2.0-only
>>>>>>     /*
>>>>>> - * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>>>>>> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>>>>>      */
>>>>>>
>>>>>>     #include <linux/clk-provider.h>
>>>>>
>>>>> [skipping]
>>>>>
>>>>>>     static struct gdsc video_cc_mvs0c_gdsc = {
>>>>>>            .gdscr = 0x804c,
>>>>>>            .en_rest_wait_val = 0x2,
>>>>>> @@ -354,15 +481,20 @@ static struct clk_regmap *video_cc_sm8550_clocks[] = {
>>>>>>            [VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
>>>>>>            [VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
>>>>>>            [VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
>>>>>> +       [VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
>>>>>>            [VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
>>>>>>            [VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
>>>>>> +       [VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
>>>>>>            [VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
>>>>>>            [VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
>>>>>>            [VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
>>>>>> +       [VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
>>>>>>            [VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
>>>>>>            [VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
>>>>>> +       [VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
>>>>>>            [VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
>>>>>>            [VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
>>>>>> +       [VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
>>>>>>     };
>>>>>>
>>>>>>     static struct gdsc *video_cc_sm8550_gdscs[] = {
>>>>>> @@ -380,6 +512,7 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
>>>>>>            [CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
>>>>>>            [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
>>>>>>            [VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
>>>>>> +       [VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },
>>>>>
>>>>> Is this reset applicable to videocc-sm8550?
>>>>>
>>>>
>>>> SM8550 also has above reset support in hardware, hence it is safe to
>>>> model above reset for both SM8550 and SM8650.
>>>
>>> Then, separate commit, Fixes tag.
>>>
>>
>> Sure, will separate and add Fixes tag in next series.
>>
>>>>
>>>>>>     };
>>>>>>
>>>>>>     static const struct regmap_config video_cc_sm8550_regmap_config = {
>>>>>> @@ -402,6 +535,7 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
>>>>>>
>>>>>>     static const struct of_device_id video_cc_sm8550_match_table[] = {
>>>>>>            { .compatible = "qcom,sm8550-videocc" },
>>>>>> +       { .compatible = "qcom,sm8650-videocc" },
>>>>>>            { }
>>>>>>     };
>>>>>>     MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
>>>>>> @@ -410,6 +544,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>>>>>     {
>>>>>>            struct regmap *regmap;
>>>>>>            int ret;
>>>>>> +       u32 offset;
>>>>>>
>>>>>>            ret = devm_pm_runtime_enable(&pdev->dev);
>>>>>>            if (ret)
>>>>>> @@ -425,6 +560,23 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>>>>>                    return PTR_ERR(regmap);
>>>>>>            }
>>>>>>
>>>>>> +       if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
>>>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
>>>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
>>>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
>>>>>> +               video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
>>>>>> +               video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
>>>>>
>>>>> Please invert the logic. Make video_cc_sm8550_clocks reflect SM8550
>>>>> and patch in new clocks in the SM8650-specific branch below.
>>>>>
>>>>
>>>> Sure, will add these clocks as NULL in video_cc_sm8550_clocks and patch
>>>> in new clocks here for SM8650. Then we can remove above check for SM8550.
>>>
>>> No need to set them to NULL, it is the default value. Just add them to
>>> the sm8650 branch.
>>>
>>
>> The video_cc_sm8550_clocks[] array size is fixed and has memory
>> allocated only for current sm8550 clocks. To be able to accommodate
>> sm8650 clocks in the same array, we need to initialize the clocks to
>> NULL as below snippet to increase the array size.
>>
>> static struct clk_regmap *video_cc_sm8550_clocks[] = {
>> .....
>>          [VIDEO_CC_XO_CLK_SRC] = NULL,
>> }
> 
> The question/comment was regarding video_cc_sm8550_probe() rather than
> video_cc_sm8550_clocks.
> 

Ok thanks, will update the change as per above comments in next series.

Thanks,
Jagadeesh

>>>>
>>>>>> +               offset = 0x8140;
>>>>>> +       } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
>>>>>> +               video_cc_pll0_config.l = 0x1e;
>>>>>> +               video_cc_pll0_config.alpha = 0xa000;
>>>>>> +               video_cc_pll1_config.l = 0x2b;
>>>>>> +               video_cc_pll1_config.alpha = 0xc000;
>>>>>> +               video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
>>>>>> +               video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
>>>>>> +               offset = 0x8150;
>>>>>> +       }
>>>>>> +
>>>>>>            clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
>>>>>>            clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
>>>>>>
>>>>>> @@ -435,7 +587,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>>>>>>             *      video_cc_xo_clk
>>>>>>             */
>>>>>>            regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
>>>>>> -       regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
>>>>>> +       regmap_update_bits(regmap, offset, BIT(0), BIT(0));
>>>>>>            regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
>>>>>>
>>>>>>            ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
>>>>>> --
>>>>>> 2.43.0
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>
>>>
>>>
> 
> 
> 

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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-06 11:31 ` [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc Jagadeesh Kona
  2024-02-06 11:54   ` Dmitry Baryshkov
@ 2024-02-07  3:49   ` kernel test robot
  2024-02-09 10:44   ` Dan Carpenter
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-02-07  3:49 UTC (permalink / raw)
  To: Jagadeesh Kona, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: llvm, oe-kbuild-all, Vladimir Zapolskiy, Taniya Das,
	linux-arm-msm, linux-clk, devicetree, linux-kernel, Imran Shaik,
	Ajit Pandey, Jagadeesh Kona

Hi Jagadeesh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on robh/for-next linus/master v6.8-rc3 next-20240206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jagadeesh-Kona/dt-bindings-clock-qcom-Add-video-clock-bindings-for-SM8650/20240206-194148
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/20240206113145.31096-3-quic_jkona%40quicinc.com
patch subject: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
config: i386-buildonly-randconfig-002-20240207 (https://download.01.org/0day-ci/archive/20240207/202402071128.aZc6BNmF-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240207/202402071128.aZc6BNmF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402071128.aZc6BNmF-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/clk/qcom/videocc-sm8550.c:570:14: warning: variable 'offset' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     570 |         } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/clk/qcom/videocc-sm8550.c:590:29: note: uninitialized use occurs here
     590 |         regmap_update_bits(regmap, offset, BIT(0), BIT(0));
         |                                    ^~~~~~
   drivers/clk/qcom/videocc-sm8550.c:570:10: note: remove the 'if' if its condition is always true
     570 |         } else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/clk/qcom/videocc-sm8550.c:547:12: note: initialize the variable 'offset' to silence this warning
     547 |         u32 offset;
         |                   ^
         |                    = 0
   1 warning generated.


vim +570 drivers/clk/qcom/videocc-sm8550.c

   542	
   543	static int video_cc_sm8550_probe(struct platform_device *pdev)
   544	{
   545		struct regmap *regmap;
   546		int ret;
   547		u32 offset;
   548	
   549		ret = devm_pm_runtime_enable(&pdev->dev);
   550		if (ret)
   551			return ret;
   552	
   553		ret = pm_runtime_resume_and_get(&pdev->dev);
   554		if (ret)
   555			return ret;
   556	
   557		regmap = qcom_cc_map(pdev, &video_cc_sm8550_desc);
   558		if (IS_ERR(regmap)) {
   559			pm_runtime_put(&pdev->dev);
   560			return PTR_ERR(regmap);
   561		}
   562	
   563		if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
   564			video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
   565			video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
   566			video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
   567			video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
   568			video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
   569			offset = 0x8140;
 > 570		} else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
   571			video_cc_pll0_config.l = 0x1e;
   572			video_cc_pll0_config.alpha = 0xa000;
   573			video_cc_pll1_config.l = 0x2b;
   574			video_cc_pll1_config.alpha = 0xc000;
   575			video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
   576			video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
   577			offset = 0x8150;
   578		}
   579	
   580		clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
   581		clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
   582	
   583		/*
   584		 * Keep clocks always enabled:
   585		 *	video_cc_ahb_clk
   586		 *	video_cc_sleep_clk
   587		 *	video_cc_xo_clk
   588		 */
   589		regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
   590		regmap_update_bits(regmap, offset, BIT(0), BIT(0));
   591		regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
   592	
   593		ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
   594	
   595		pm_runtime_put(&pdev->dev);
   596	
   597		return ret;
   598	}
   599	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
  2024-02-06 11:31 ` [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc Jagadeesh Kona
  2024-02-06 11:54   ` Dmitry Baryshkov
  2024-02-07  3:49   ` kernel test robot
@ 2024-02-09 10:44   ` Dan Carpenter
  2 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2024-02-09 10:44 UTC (permalink / raw)
  To: oe-kbuild, Jagadeesh Kona, Bjorn Andersson, Konrad Dybcio,
	Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: lkp, oe-kbuild-all, Vladimir Zapolskiy, Taniya Das, linux-arm-msm,
	linux-clk, devicetree, linux-kernel, Imran Shaik, Ajit Pandey,
	Jagadeesh Kona

Hi Jagadeesh,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jagadeesh-Kona/dt-bindings-clock-qcom-Add-video-clock-bindings-for-SM8650/20240206-194148
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/20240206113145.31096-3-quic_jkona%40quicinc.com
patch subject: [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc
config: arm64-randconfig-r071-20240207 (https://download.01.org/0day-ci/archive/20240209/202402091804.SdrSLt10-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202402091804.SdrSLt10-lkp@intel.com/

smatch warnings:
drivers/clk/qcom/videocc-sm8550.c:590 video_cc_sm8550_probe() error: uninitialized symbol 'offset'.

vim +/offset +590 drivers/clk/qcom/videocc-sm8550.c

f53153a37969c1 Jagadeesh Kona   2023-05-24  543  static int video_cc_sm8550_probe(struct platform_device *pdev)
f53153a37969c1 Jagadeesh Kona   2023-05-24  544  {
f53153a37969c1 Jagadeesh Kona   2023-05-24  545  	struct regmap *regmap;
f53153a37969c1 Jagadeesh Kona   2023-05-24  546  	int ret;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  547  	u32 offset;
f53153a37969c1 Jagadeesh Kona   2023-05-24  548  
f53153a37969c1 Jagadeesh Kona   2023-05-24  549  	ret = devm_pm_runtime_enable(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  550  	if (ret)
f53153a37969c1 Jagadeesh Kona   2023-05-24  551  		return ret;
f53153a37969c1 Jagadeesh Kona   2023-05-24  552  
f53153a37969c1 Jagadeesh Kona   2023-05-24  553  	ret = pm_runtime_resume_and_get(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  554  	if (ret)
f53153a37969c1 Jagadeesh Kona   2023-05-24  555  		return ret;
f53153a37969c1 Jagadeesh Kona   2023-05-24  556  
f53153a37969c1 Jagadeesh Kona   2023-05-24  557  	regmap = qcom_cc_map(pdev, &video_cc_sm8550_desc);
f53153a37969c1 Jagadeesh Kona   2023-05-24  558  	if (IS_ERR(regmap)) {
f53153a37969c1 Jagadeesh Kona   2023-05-24  559  		pm_runtime_put(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  560  		return PTR_ERR(regmap);
f53153a37969c1 Jagadeesh Kona   2023-05-24  561  	}
f53153a37969c1 Jagadeesh Kona   2023-05-24  562  
5ab3df7257a04f Jagadeesh Kona   2024-02-06  563  	if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8550-videocc")) {
5ab3df7257a04f Jagadeesh Kona   2024-02-06  564  		video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  565  		video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  566  		video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  567  		video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  568  		video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = NULL;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  569  		offset = 0x8140;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  570  	} else  if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
5ab3df7257a04f Jagadeesh Kona   2024-02-06  571  		video_cc_pll0_config.l = 0x1e;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  572  		video_cc_pll0_config.alpha = 0xa000;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  573  		video_cc_pll1_config.l = 0x2b;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  574  		video_cc_pll1_config.alpha = 0xc000;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  575  		video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  576  		video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  577  		offset = 0x8150;
5ab3df7257a04f Jagadeesh Kona   2024-02-06  578  	}

no else statement.

5ab3df7257a04f Jagadeesh Kona   2024-02-06  579  
a2620539ae2529 Dmitry Baryshkov 2023-10-16  580  	clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
a2620539ae2529 Dmitry Baryshkov 2023-10-16  581  	clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
f53153a37969c1 Jagadeesh Kona   2023-05-24  582  
f53153a37969c1 Jagadeesh Kona   2023-05-24  583  	/*
f53153a37969c1 Jagadeesh Kona   2023-05-24  584  	 * Keep clocks always enabled:
f53153a37969c1 Jagadeesh Kona   2023-05-24  585  	 *	video_cc_ahb_clk
f53153a37969c1 Jagadeesh Kona   2023-05-24  586  	 *	video_cc_sleep_clk
f53153a37969c1 Jagadeesh Kona   2023-05-24  587  	 *	video_cc_xo_clk
f53153a37969c1 Jagadeesh Kona   2023-05-24  588  	 */
f53153a37969c1 Jagadeesh Kona   2023-05-24  589  	regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
5ab3df7257a04f Jagadeesh Kona   2024-02-06 @590  	regmap_update_bits(regmap, offset, BIT(0), BIT(0));
f53153a37969c1 Jagadeesh Kona   2023-05-24  591  	regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
f53153a37969c1 Jagadeesh Kona   2023-05-24  592  
f53153a37969c1 Jagadeesh Kona   2023-05-24  593  	ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
f53153a37969c1 Jagadeesh Kona   2023-05-24  594  
f53153a37969c1 Jagadeesh Kona   2023-05-24  595  	pm_runtime_put(&pdev->dev);
f53153a37969c1 Jagadeesh Kona   2023-05-24  596  
f53153a37969c1 Jagadeesh Kona   2023-05-24  597  	return ret;
f53153a37969c1 Jagadeesh Kona   2023-05-24  598  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


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

end of thread, other threads:[~2024-02-14  5:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-09 10:39 [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2024-02-06 11:31 [PATCH 0/5] Add support for videocc and camcc on SM8650 Jagadeesh Kona
2024-02-06 11:31 ` [PATCH 2/5] clk: qcom: videocc-sm8550: Add support for SM8650 videocc Jagadeesh Kona
2024-02-06 11:54   ` Dmitry Baryshkov
2024-02-07  6:58     ` Jagadeesh Kona
2024-02-07  7:19       ` Dmitry Baryshkov
2024-02-12 13:06         ` Jagadeesh Kona
2024-02-12 13:18           ` Dmitry Baryshkov
2024-02-14  5:29             ` Jagadeesh Kona
2024-02-07  3:49   ` kernel test robot
2024-02-09 10:44   ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.