Messages in this thread | | | Date | Fri, 19 May 2023 16:23:03 +0530 | Subject | Re: [PATCH V4 2/3] clk: qcom: videocc-sm8450: Add video clock controller driver for SM8450 | From | Taniya Das <> |
| |
Hello Dmitry,
Thank you for your review.
On 5/10/2023 2:03 AM, Dmitry Baryshkov wrote: > On Tue, 9 May 2023 at 20:22, 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> >> --- >> Changes since V3: >> - Use lower case hex. >> - Check the return value here and bail out early on failure in probe. >> >> Changes since V2: >> - Update the header file name to match the latest upstream header >> files. >> >> Changes since V1: >> - Use DT indices instead of fw_name. >> - Replace pm_runtime_enable with devm_pm_runtime_enable. >> - Change license to GPL from GPL V2. >> >> drivers/clk/qcom/Kconfig | 9 + >> drivers/clk/qcom/Makefile | 1 + >> drivers/clk/qcom/videocc-sm8450.c | 461 ++++++++++++++++++++++++++++++ >> 3 files changed, 471 insertions(+) >> create mode 100644 drivers/clk/qcom/videocc-sm8450.c > > [skipped] > > >> +static const struct qcom_reset_map video_cc_sm8450_resets[] = { >> + [CVP_VIDEO_CC_INTERFACE_BCR] = { 0x80e0 }, >> + [CVP_VIDEO_CC_MVS0_BCR] = { 0x8098 }, >> + [CVP_VIDEO_CC_MVS0C_BCR] = { 0x8048 }, >> + [CVP_VIDEO_CC_MVS1_BCR] = { 0x80bc }, >> + [CVP_VIDEO_CC_MVS1C_BCR] = { 0x8070 }, > > Can we have a common VIDEO_CC prefix here please?
The BCR names are coming from hardware plan and software interface, thus we would like to keep them intact.
> >> + [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 }, >> + [VIDEO_CC_MVS1C_CLK_ARES] = { 0x808c, 2 }, >> +}; >> +
The ARES resets are coming from VideoCC clocks(CBCR), hence the name starts with VIDEO_CC.
>> +static const struct regmap_config video_cc_sm8450_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 32, >> + .max_register = 0x9f4c, >> + .fast_io = true, >> +}; >> + >> +static struct qcom_cc_desc video_cc_sm8450_desc = { >> + .config = &video_cc_sm8450_regmap_config, >> + .clks = video_cc_sm8450_clocks, >> + .num_clks = ARRAY_SIZE(video_cc_sm8450_clocks), >> + .resets = video_cc_sm8450_resets, >> + .num_resets = ARRAY_SIZE(video_cc_sm8450_resets), >> + .gdscs = video_cc_sm8450_gdscs, >> + .num_gdscs = ARRAY_SIZE(video_cc_sm8450_gdscs), >> +}; >> + >> +static const struct of_device_id video_cc_sm8450_match_table[] = { >> + { .compatible = "qcom,sm8450-videocc" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, video_cc_sm8450_match_table); >> + >> +static int video_cc_sm8450_probe(struct platform_device *pdev) >> +{ >> + struct regmap *regmap; >> + int ret; >> + >> + ret = devm_pm_runtime_enable(&pdev->dev); >> + if (ret) >> + return ret; >> + >> + 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_platform_driver() ? >
We would like to keep the clock drivers all probed at subsys_initcall. We could revisit and update as cleanup if we want to move them to module init.
>> + >> +MODULE_DESCRIPTION("QTI VIDEO_CC SM8450 Driver"); >> +MODULE_LICENSE("GPL"); >> -- >> 2.17.1 >> > >
-- Thanks & Regards, Taniya Das.
| |