linux-kernel - Re: [PATCH v2 5/6] clk: qcom: Add camera clock controller driver for SM8150

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: <0b84b689-8ab8-bcdf-f058-da2ead73786c@quicinc.com>
Date: Wed, 17 Jul 2024 11:54:33 +0530
From: "Satya Priya Kakitapalli (Temp)" <quic_skakitap@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Dmitry Baryshkov
	<dmitry.baryshkov@...aro.org>
CC: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konrad.dybcio@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, Abhishek Sahu <absahu@...eaurora.org>,
        "Rob
 Herring" <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Stephen Boyd <sboyd@...eaurora.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-clk@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <devicetree@...r.kernel.org>, Ajit Pandey <quic_ajipan@...cinc.com>,
        "Imran
 Shaik" <quic_imrashai@...cinc.com>,
        Taniya Das <quic_tdas@...cinc.com>,
        Jagadeesh Kona <quic_jkona@...cinc.com>
Subject: Re: [PATCH v2 5/6] clk: qcom: Add camera clock controller driver for
 SM8150


On 7/15/2024 8:31 PM, Bryan O'Donoghue wrote:
> On 15/07/2024 11:38, Dmitry Baryshkov wrote:
>>>> Does it apply to SM8150? For example, on SM8250 RCG2s are not parked.
>>>
>>> Yes, it applies to SM8150.
>> Should the same logic be applied to other chipsets supported upstream?
>> If this is the case, which chipsets?
>
> If you are representing the "top" GDSC inside of the CCF instead of 
> doing this
>
> +    /* Keep the critical clock always-on */
> +    qcom_branch_set_clk_en(regmap, 0xc1e4); /* cam_cc_gdsc_clk */
>
> then the clock should be parked else you'll find the GDSC doesn't come 
> out of reset.
>
> and... as I look at it now we have a logical conflict in 
> drivers/clk/qcom/camcc-sc8280xp.c
>
> static struct clk_branch camcc_gdsc_clk = {
>         .halt_reg = 0xc1e4,
>         .halt_check = BRANCH_HALT,
>         .clkr = {
>                 .enable_reg = 0xc1e4,
>                 .enable_mask = BIT(0),
>                 .hw.init = &(struct clk_init_data){
>                         .name = "camcc_gdsc_clk",
>                         .parent_hws = (const struct clk_hw*[]){
>                                 &camcc_xo_clk_src.clkr.hw,
>                         },
>                         .num_parents = 1,
>                         .flags = CLK_SET_RATE_PARENT,
>                         .ops = &clk_branch2_ops,
>                 },
>         },
> };
>
> Patch sent.
>
> https://lore.kernel.org/linux-arm-msm/20240715-linux-next-24-07-13-sc8280xp-camcc-fixes-v1-1-fadb5d9445c1@linaro.org/T/#u 
>
>

If the clock is modelled, it can get disabled during the late init call, 
when the CCF disables the unused clocks. But, it is a PoR ON  clock and 
expectation from design team is to keep it always-on for GDSC functionality.

So, we should not model this, instead just keep it always on from probe.


> In the round I think we should avoid these horrific hard-coded 
> always-on writes where possible.
>

We have been suggested to make such clocks always ON from probe on 
previous upstream discussions. This is also recently acknowledged by 
maintainers, please see [1] and [2].


[1] 
https://lore.kernel.org/linux-clk/6286a410-6faa-4996-8a9e-dc334dd9421f@linaro.org/


[2] 
https://lore.kernel.org/linux-clk/664cca91-8615-d3f6-7525-15b9b6725cce@quicinc.com/


> Personally I think parking is better than always-on specifically 
> because you define the clock and "see" it in the clock tree.
>
> ---
> bod