| Message ID | 20240206113145.31096-3-quic_jkona@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for videocc and camcc on SM8650 | expand |
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 > >
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
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 >> >> > >
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 > >> > >> > > > >
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 }
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 >>>> >>>> >>> >>> > > >
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 > >>>> > >>>> > >>> > >>> > > > > > >
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 >>>>>> >>>>>> >>>>> >>>>> >>> >>> >>> > > >
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);
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(-)