Re: [PATCH V2 7/8] clk: qcom: Add GPUCC driver support for SM4450

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/26/2024 3:05 PM, Dmitry Baryshkov wrote:
On Fri, 26 Apr 2024 at 12:20, Ajit Pandey <quic_ajipan@xxxxxxxxxxx> wrote:



On 4/17/2024 11:35 AM, Dmitry Baryshkov wrote:
On Tue, 16 Apr 2024 at 21:23, Ajit Pandey <quic_ajipan@xxxxxxxxxxx> wrote:

Add Graphics Clock Controller (GPUCC) support for SM4450 platform.

Signed-off-by: Ajit Pandey <quic_ajipan@xxxxxxxxxxx>
---
   drivers/clk/qcom/Kconfig        |   9 +
   drivers/clk/qcom/Makefile       |   1 +
   drivers/clk/qcom/gpucc-sm4450.c | 805 ++++++++++++++++++++++++++++++++
   3 files changed, 815 insertions(+)
   create mode 100644 drivers/clk/qcom/gpucc-sm4450.c

[skipped]

+
+static int gpu_cc_sm4450_probe(struct platform_device *pdev)
+{
+       struct regmap *regmap;
+
+       regmap = qcom_cc_map(pdev, &gpu_cc_sm4450_desc);
+       if (IS_ERR(regmap))
+               return PTR_ERR(regmap);
+
+       clk_lucid_evo_pll_configure(&gpu_cc_pll0, regmap, &gpu_cc_pll0_config);
+       clk_lucid_evo_pll_configure(&gpu_cc_pll1, regmap, &gpu_cc_pll1_config);
+
+       /* Keep some clocks always enabled */
+       qcom_branch_set_clk_en(regmap, 0x93a4); /* GPU_CC_CB_CLK */
+       qcom_branch_set_clk_en(regmap, 0x9004); /* GPU_CC_CXO_AON_CLK */
+       qcom_branch_set_clk_en(regmap, 0x900c); /* GPU_CC_DEMET_CLK */

My main concern here is the AON clocks. If we don't model
gpu_cc_demet_clk as a leaf clock, then gpu_cc_demet_div_clk_src
becomes a clock without children and can be disabled by Linux.
Likewise not modelling gpu_cc_cxo_aon_clk removes one of the voters on
gpu_cc_xo_clk_src, which can now be turned off by Linux.
Our usual recommendation is to model such clocks properly and to use
CLK_IS_CRITICAL or CLK_IGNORE_UNUSED to mark then as aon.

Thanks for review, actually if leaf (branch) clock is ON, hardware will
take care of enabling and keeping the parent ON. So parent clocks won't
get turned OFF in HW as long as branch clock is enabled.

For clocks which are fixed rate (19.2MHz) and recommended to be kept ON
forever from HW design, modelling and exposing clock structure in kernel
will be a redundant code in kernel memory, hence as per earlier
suggestion in previous thread such clocks are recommended to be kept
enabled from probe.

Recommended by whom?

Kernel developers clearly recommend describing all the clocks so that
CCF has knowledge about all the clocks in the system.

Actually it's been recommended earlier by Stephen during initial discussion on moving such critical clocks to probe to avoid redundant codes in kernel memory. From then we're following similar approach in other mainlined CC's drivers for fixed rate clocks which needs to kept enabled always - eg: DISP_CC_XO_CLK (keeping bits enabled in probe) in SM8450, SM8650 etc.


+
+       return qcom_cc_really_probe(pdev, &gpu_cc_sm4450_desc, regmap);
+}
+
+static struct platform_driver gpu_cc_sm4450_driver = {
+       .probe = gpu_cc_sm4450_probe,
+       .driver = {
+               .name = "gpucc-sm4450",
+               .of_match_table = gpu_cc_sm4450_match_table,
+       },
+};
+
+module_platform_driver(gpu_cc_sm4450_driver);
+
+MODULE_DESCRIPTION("QTI GPUCC SM4450 Driver");
+MODULE_LICENSE("GPL");
--
2.25.1





--
Thanks, and Regards
Ajit




--
Thanks, and Regards
Ajit




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux