linux-kernel - Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <436cc6a3-7406-c695-7879-3b9d042262cc@codeaurora.org>
Date:   Mon, 9 Jul 2018 12:37:21 +0530
From:   Taniya Das <tdas@...eaurora.org>
To:     Stephen Boyd <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>
Cc:     Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Amit Nischal <anischal@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for
 SDM845



On 7/9/2018 11:46 AM, Stephen Boyd wrote:
> Quoting Taniya Das (2018-07-08 20:38:03)
>> Hello Stephen,
>>
>> Thanks for your review comments.
>>
>> On 7/9/2018 5:24 AM, Stephen Boyd wrote:
>>> Quoting Taniya Das (2018-06-23 07:19:27)
>>>> diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-sdm845.c
>>>> new file mode 100644
>>>> index 0000000..af437e0
>>>> --- /dev/null
>>>> +++ b/drivers/clk/qcom/dispcc-sdm845.c
>>>> @@ -0,0 +1,674 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>> [...]
>>>> +static struct clk_alpha_pll disp_cc_pll0 = {
>>>> +       .offset = 0x0,
>>>> +       .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
>>>> +       .clkr = {
>>>> +               .hw.init = &(struct clk_init_data){
>>>> +                       .name = "disp_cc_pll0",
>>>> +                       .parent_names = (const char *[]){ "bi_tcxo" },
>>>> +                       .num_parents = 1,
>>>> +                       .ops = &clk_alpha_pll_fabia_ops,
>>>> +               },
>>>> +       },
>>>> +};
>>>> +
>>>> +static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = {
>>>> +       .cmd_rcgr = 0x20d0,
>>>> +       .mnd_width = 0,
>>>> +       .hid_width = 5,
>>>> +       .parent_map = disp_cc_parent_map_0,
>>>> +       .clkr.hw.init = &(struct clk_init_data){
>>>> +               .name = "disp_cc_mdss_byte0_clk_src",
>>>> +               .parent_names = disp_cc_parent_names_0,
>>>> +               .num_parents = 4,
>>>> +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
>>>
>>> Why is there the no cache flag? Last time I asked I don't think I got
>>> any answer, and there isn't a comment here so please at least add a
>>> comment to the code so we don't forget.
>>>
>>
>> I think you missed my comment from the earlier email. I would add the
>> comment and submit again.
> 
> Hmm.. ok.
> 
>>
>>   > Why is the nocache flag needed? Applies to all clks in this file.
>>   >
>>
>> This flag is required for all RCGs whose PLLs are controlled outside the
>> clock controller. The display code would require the recalculated rate
>> always.
> 
> Right. Why is the PLL controlled outside of the clock controller? The
> rate should propagate upward to the PLL from here, so who's going
> outside of that?
> 
The DSI0/1 PLL are not part of the display clock controller, but in the 
display subsystem which are managed by the DRM drivers. When DRM drivers
query for the rate clock driver should always return the non cached rates.
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--