Linux-Clk Archive mirror
 help / color / mirror / Atom feed
From: Ajit Pandey <quic_ajipan@quicinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Vinod Koul <vkoul@kernel.org>,
	Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>,
	<linux-arm-msm@vger.kernel.org>, <linux-clk@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Taniya Das <quic_tdas@quicinc.com>,
	"Jagadeesh Kona" <quic_jkona@quicinc.com>,
	Imran Shaik <quic_imrashai@quicinc.com>,
	"Satya Priya Kakitapalli" <quic_skakitap@quicinc.com>
Subject: Re: [PATCH V2 7/8] clk: qcom: Add GPUCC driver support for SM4450
Date: Tue, 30 Apr 2024 16:31:06 +0530	[thread overview]
Message-ID: <2679710d-46a9-8544-afff-8a406fdde918@quicinc.com> (raw)
In-Reply-To: <CAA8EJporZFsjagW5CU5AwtqDsEXTtGJmRmLRedyBTZa7249p6w@mail.gmail.com>



On 4/26/2024 3:05 PM, Dmitry Baryshkov wrote:
> On Fri, 26 Apr 2024 at 12:20, Ajit Pandey <quic_ajipan@quicinc.com> wrote:
>>
>>
>>
>> On 4/17/2024 11:35 AM, Dmitry Baryshkov wrote:
>>> On Tue, 16 Apr 2024 at 21:23, Ajit Pandey <quic_ajipan@quicinc.com> wrote:
>>>>
>>>> Add Graphics Clock Controller (GPUCC) support for SM4450 platform.
>>>>
>>>> Signed-off-by: Ajit Pandey <quic_ajipan@quicinc.com>
>>>> ---
>>>>    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

  reply	other threads:[~2024-04-30 11:01 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-16 18:19 [PATCH V2 0/8] clk: qcom: Add support for DISPCC, CAMCC and GPUCC on SM4450 Ajit Pandey
2024-04-16 18:19 ` [PATCH V2 1/8] clk: qcom: clk-alpha-pll: Fix CAL_L_VAL override for LUCID EVO PLL Ajit Pandey
2024-04-16 18:52   ` Dmitry Baryshkov
2024-04-16 18:19 ` [PATCH V2 2/8] dt-bindings: clock: qcom: add bindings for dispcc on SM4450 Ajit Pandey
2024-04-16 18:20 ` [PATCH V2 3/8] clk: qcom: Add DISPCC driver support for SM4450 Ajit Pandey
2024-04-16 18:56   ` Dmitry Baryshkov
2024-04-16 18:20 ` [PATCH V2 4/8] dt-bindings: clock: qcom: add bindings for camcc on SM4450 Ajit Pandey
2024-04-17 18:50   ` Rob Herring
2024-04-16 18:20 ` [PATCH V2 5/8] clk: qcom: Add CAMCC driver support for SM4450 Ajit Pandey
2024-04-16 18:20 ` [PATCH V2 6/8] dt-bindings: clock: qcom: add bindings for gpucc on SM4450 Ajit Pandey
2024-04-17 19:26   ` Krzysztof Kozlowski
2024-04-17 19:27     ` Krzysztof Kozlowski
2024-04-17 19:28   ` Krzysztof Kozlowski
2024-04-16 18:20 ` [PATCH V2 7/8] clk: qcom: Add GPUCC driver support for SM4450 Ajit Pandey
2024-04-17  6:05   ` Dmitry Baryshkov
2024-04-26  9:20     ` Ajit Pandey
2024-04-26  9:35       ` Dmitry Baryshkov
2024-04-30 11:01         ` Ajit Pandey [this message]
2024-04-16 18:20 ` [PATCH V2 8/8] arm64: dts: qcom: sm4450: add camera, display and gpu clock controller Ajit Pandey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2679710d-46a9-8544-afff-8a406fdde918@quicinc.com \
    --to=quic_ajipan@quicinc.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=konrad.dybcio@linaro.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=quic_imrashai@quicinc.com \
    --cc=quic_jkona@quicinc.com \
    --cc=quic_skakitap@quicinc.com \
    --cc=quic_tdas@quicinc.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=vkoul@kernel.org \
    --cc=vladimir.zapolskiy@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).