Messages in this thread | | | Date | Mon, 17 Apr 2023 10:07:06 +0530 | Subject | Re: [PATCH 2/3] clk: qcom: videocc-sm8450: Add video clock controller driver for SM8450 | From | Taniya Das <> |
| |
Hi Dmitry,
Thank you for the comments.
On 3/16/2023 3:41 PM, Dmitry Baryshkov wrote: > On Thu, 16 Mar 2023 at 10:31, Taniya Das <quic_tdas@quicinc.com> wrote: >> >> Add support for the video clock controller driver for peripheral clock >> clients to be able to request for video cc clocks. >> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >> --- >> drivers/clk/qcom/Kconfig | 9 + >> drivers/clk/qcom/Makefile | 1 + >> drivers/clk/qcom/videocc-sm8450.c | 464 ++++++++++++++++++++++++++++++ >> 3 files changed, 474 insertions(+) >> create mode 100644 drivers/clk/qcom/videocc-sm8450.c >> >> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig >> index 5ab4b7dfe3c2..81909e179bc7 100644 >> --- a/drivers/clk/qcom/Kconfig >> +++ b/drivers/clk/qcom/Kconfig >> @@ -892,4 +892,13 @@ config CLK_GFM_LPASS_SM8250 >> Support for the Glitch Free Mux (GFM) Low power audio >> subsystem (LPASS) clocks found on SM8250 SoCs. >> >> +config SM_VIDEOCC_8450 >> + tristate "SM8450 Video Clock Controller" >> + select SM_GCC_8450 >> + select QCOM_GDSC >> + help >> + Support for the video clock controller on Qualcomm Technologies, Inc. >> + SM8450 devices. >> + Say Y if you want to support video devices and functionality such as >> + video encode/decode. >> endif >> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile >> index c743805a9cbb..5cbd0eedd6d9 100644 >> --- a/drivers/clk/qcom/Makefile >> +++ b/drivers/clk/qcom/Makefile >> @@ -119,6 +119,7 @@ obj-$(CONFIG_SM_GPUCC_8350) += gpucc-sm8350.o >> obj-$(CONFIG_SM_TCSRCC_8550) += tcsrcc-sm8550.o >> obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o >> obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o >> +obj-$(CONFIG_SM_VIDEOCC_8450) += videocc-sm8450.o >> obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o >> obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o >> obj-$(CONFIG_QCOM_HFPLL) += hfpll.o >> diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c >> new file mode 100644 >> index 000000000000..ca60f3be587d >> --- /dev/null >> +++ b/drivers/clk/qcom/videocc-sm8450.c >> @@ -0,0 +1,464 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/module.h> >> +#include <linux/of_device.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/regmap.h> >> + >> +#include <dt-bindings/clock/qcom,videocc-sm8450.h> >> + >> +#include "clk-alpha-pll.h" >> +#include "clk-branch.h" >> +#include "clk-rcg.h" >> +#include "clk-regmap.h" >> +#include "clk-regmap-divider.h" >> +#include "common.h" >> +#include "gdsc.h" >> +#include "reset.h" >> + >> +enum { >> + P_BI_TCXO, >> + P_VIDEO_CC_PLL0_OUT_MAIN, >> + P_VIDEO_CC_PLL1_OUT_MAIN, >> +}; >> + >> +static const struct pll_vco lucid_evo_vco[] = { >> + { 249600000, 2020000000, 0 }, >> +}; >> + >> +static const struct alpha_pll_config video_cc_pll0_config = { >> + .l = 0x1E, >> + .alpha = 0x0, >> + .config_ctl_val = 0x20485699, >> + .config_ctl_hi_val = 0x00182261, >> + .config_ctl_hi1_val = 0x32AA299C, >> + .user_ctl_val = 0x00000000, >> + .user_ctl_hi_val = 0x00000805, >> +}; >> + >> +static struct clk_alpha_pll video_cc_pll0 = { >> + .offset = 0x0, >> + .vco_table = lucid_evo_vco, >> + .num_vco = ARRAY_SIZE(lucid_evo_vco), >> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO], >> + .clkr = { >> + .hw.init = &(const struct clk_init_data){ >> + .name = "video_cc_pll0", >> + .parent_data = &(const struct clk_parent_data){ >> + .fw_name = "bi_tcxo", > > Could you please follow the last of recent drivers and use DT indices > instead of clock-names? >
Thanks, will fix them in the next patch set.
> Also, as a syntax nit, could you please add whitespaces between ) and { ? >
Done.
>> + }, >> + .num_parents = 1, >> + .ops = &clk_alpha_pll_lucid_evo_ops, >> + }, >> + }, >> +}; >> + > > [skipped] > > >> + >> +static void video_cc_sm8450_pm_runtime_disable(void *data) >> +{ >> + pm_runtime_disable(data); >> +} >> + >> +static int video_cc_sm8450_probe(struct platform_device *pdev) >> +{ >> + struct regmap *regmap; >> + int ret; >> + >> + pm_runtime_enable(&pdev->dev); >> + >> + ret = devm_add_action_or_reset(&pdev->dev, video_cc_sm8450_pm_runtime_disable, &pdev->dev); >> + if (ret) >> + return ret; > > Could you please shift to using devm_pm_runtime_enable()? >
Done.
>> + >> + ret = pm_runtime_resume_and_get(&pdev->dev); >> + if (ret) >> + return ret; >> + >> + regmap = qcom_cc_map(pdev, &video_cc_sm8450_desc); >> + if (IS_ERR(regmap)) { >> + pm_runtime_put(&pdev->dev); >> + return PTR_ERR(regmap); >> + } >> + >> + clk_lucid_evo_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config); >> + clk_lucid_evo_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config); >> + >> + /* >> + * Keep clocks always enabled: >> + * video_cc_ahb_clk >> + * video_cc_sleep_clk >> + * video_cc_xo_clk >> + */ >> + regmap_update_bits(regmap, 0x80e4, BIT(0), BIT(0)); >> + regmap_update_bits(regmap, 0x8130, BIT(0), BIT(0)); >> + regmap_update_bits(regmap, 0x8114, BIT(0), BIT(0)); >> + >> + ret = qcom_cc_really_probe(pdev, &video_cc_sm8450_desc, regmap); >> + >> + pm_runtime_put(&pdev->dev); >> + >> + return ret; >> +} >> + >> +static struct platform_driver video_cc_sm8450_driver = { >> + .probe = video_cc_sm8450_probe, >> + .driver = { >> + .name = "video_cc-sm8450", >> + .of_match_table = video_cc_sm8450_match_table, >> + }, >> +}; >> + >> +static int __init video_cc_sm8450_init(void) >> +{ >> + return platform_driver_register(&video_cc_sm8450_driver); >> +} >> +subsys_initcall(video_cc_sm8450_init); >> + >> +static void __exit video_cc_sm8450_exit(void) >> +{ >> + platform_driver_unregister(&video_cc_sm8450_driver); >> +} >> +module_exit(video_cc_sm8450_exit); >> + >> +MODULE_DESCRIPTION("QTI VIDEO_CC SM8450 Driver"); >> +MODULE_LICENSE("GPL v2"); > > I think this should be just "GPL" nowaways. >
Will take care in the next patch set.
>> -- >> 2.17.1 >> > >
-- Thanks & Regards, Taniya Das.
| |