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

[Ajit Pandey]

On 4/3/2024 12:53 AM, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 21:26, Ajit Pandey <quic_ajipan@xxxxxxxxxxx> wrote:
> > On 3/31/2024 7:09 AM, Dmitry Baryshkov wrote:
> > > On Sat, 30 Mar 2024 at 20:30, 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 | 806 ++++++++++++++++++++++++++++++++
> > > >    3 files changed, 816 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 */
> > >
> > > Why? At least other drivers model these three clocks properly.
> > These clocks are POR on in SM4450 and required to be kept always enabled
> > for GPU functionality hence keep them enabled from probe only.
>
> Please, check how this is handled on the other platforms, please.
> Hint: `git grep GPU_CC_DEMET_CLK`
yeah these clocks are modeled and handled via always enabled clk ops 
(clk_branch2_aon_ops) in few other platforms like SM8450, SM8650 which 
also do same functionality and keep them in always enabled POR state, 
while we kept them enabled from GPUCC probe in SM8550.
Since we need such clock to be always enabled irrespective of consumer 
votes I guess modeling with aon_ops isn't really required and we can 
simply keep them enabled in probe similar to other always on clocks.
> > > > +
> > > > +       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